[PATCH 00/21] MIPS: KVM: Fixes and guest timer rewrite

[PATCH 00/21] MIPS: KVM: Fixes and guest timer rewrite

[James Hogan]

Here are a range of MIPS KVM T&E fixes for v3.16. They can also be found
on my kvm_mips_queue branch here:
git://git.kernel.org/pub/scm/linux/kernel/git/jhogan/kvm-mips.git

They originally served to allow it to work better on Ingenic XBurst
cores which have some peculiarities which break non-portable assumptions
in the MIPS KVM implementation (patches 1-3, 11).

Fixing guest CP0_Count emulation to work without a running host
CP0_Count (patch 11) however required a rewrite of the timer emulation
code to use the kernel monotonic time instead, which needed doing anyway
since basing it directly off the host CP0_Count was broken. Various bugs
were fixed in the process (patches 8-10) and improvements made thanks to
valuable feedback from Paolo Bonzini for the last QEMU MIPS/KVM patchset
(patches 4-7, 13-15).

Finally there are some misc cleanups which I did along the way (patches
16-21).

Only the first patch (fixes MIPS KVM with 4K pages) is marked for
stable. For KVM to work on XBurst it needs the timer rework which is a
fairly complex change, so there's little point marking any of the XBurst
specific changes for stable.

All feedback welcome!

Patches 1-3:
	Fix KVM/MIPS with 4K pages, missing RDHWR SYNCI (XBurst),
	unmoving CP0_Random (XBurst).
Patches 4-7:
	Add EPC, Count, Compare guest CP0 registers to KVM register
	ioctl interface.
Patches 8-10:
	Fix a few potential races relating to timers.
Patches 11-12:
	Rewrite guest timer emulation to use ktime_get().
Patches 13-15:
	Add KVM virtual registers for controlling guest timer, including
	master timer disable, nanosecond bias, and timer frequency.
Patches 16-21:
	Cleanups.

James Hogan (21):
  MIPS: KVM: Allocate at least 16KB for exception handlers
  MIPS: KVM: Use local_flush_icache_range to fix RI on XBurst
  MIPS: KVM: Use tlb_write_random
  MIPS: KVM: Fix CP0_EBASE KVM register id
  MIPS: KVM: Add CP0_EPC KVM register access
  MIPS: KVM: Move KVM_{GET,SET}_ONE_REG definitions into kvm_host.h
  MIPS: KVM: Add CP0_Count/Compare KVM register access
  MIPS: KVM: Deliver guest interrupts after local_irq_disable()
  MIPS: KVM: Fix timer race modifying guest CP0_Cause
  MIPS: KVM: Migrate hrtimer to follow VCPU
  MIPS: KVM: Rewrite count/compare timer emulation
  MIPS: KVM: Override guest kernel timer frequency directly
  MIPS: KVM: Add master disable count interface
  MIPS: KVM: Add nanosecond count bias KVM register
  MIPS: KVM: Add count frequency KVM register
  MIPS: KVM: Make kvm_mips_comparecount_{func,wakeup} static
  MIPS: KVM: Whitespace fixes in kvm_mips_callbacks
  MIPS: KVM: Fix kvm_debug bit-rottage
  MIPS: KVM: Remove ifdef DEBUG around kvm_debug
  MIPS: KVM: Quieten kvm_info() logging
  MIPS: KVM: Remove redundant NULL checks before kfree()

 arch/mips/Kconfig                 |  12 +-
 arch/mips/include/asm/kvm_host.h  | 185 +++++++++---
 arch/mips/include/uapi/asm/kvm.h  |  40 +++
 arch/mips/kvm/kvm_locore.S        |  32 --
 arch/mips/kvm/kvm_mips.c          | 127 ++++----
 arch/mips/kvm/kvm_mips_dyntrans.c |  15 +-
 arch/mips/kvm/kvm_mips_emul.c     | 608 ++++++++++++++++++++++++++++++++++++--
 arch/mips/kvm/kvm_tlb.c           |  60 ++--
 arch/mips/kvm/kvm_trap_emul.c     |  89 +++++-
 arch/mips/mti-malta/malta-time.c  |  14 +-
 10 files changed, 951 insertions(+), 231 deletions(-)

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: David Daney <david.daney@cavium.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
-- 
1.8.1.2

[PATCH 01/21] MIPS: KVM: Allocate at least 16KB for exception handlers

[James Hogan]

Each MIPS KVM guest has its own copy of the KVM exception vector. This
contains the TLB refill exception handler at offset 0x000, the general
exception handler at offset 0x180, and interrupt exception handlers at
offset 0x200 in case Cause_IV=1. A common handler is copied to offset
0x2000 and offset 0x3000 is used for temporarily storing k1 during entry
from guest.

However the amount of memory allocated for this purpose is calculated as
0x200 rounded up to the next page boundary, which is insufficient if 4KB
pages are in use. This can lead to the common handler at offset 0x2000
being overwritten and infinitely recursive exceptions on the next exit
from the guest.

Increase the minimum size from 0x200 to 0x4000 to cover the full use of
the page.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
Cc: stable@vger.kernel.org
---
 arch/mips/kvm/kvm_mips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index da5186fbd77a..5efce56f0df0 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -304,7 +304,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	if (cpu_has_veic || cpu_has_vint) {
 		size = 0x200 + VECTORSPACING * 64;
 	} else {
-		size = 0x200;
+		size = 0x4000;
 	}
 
 	/* Save Linux EBASE */
-- 
1.8.1.2

[PATCH 02/21] MIPS: KVM: Use local_flush_icache_range to fix RI on XBurst

[James Hogan]

MIPS KVM uses mips32_SyncICache to synchronise the icache with the
dcache after dynamically modifying guest instructions or writing guest
exception vector. However this uses rdhwr to get the SYNCI step, which
causes a reserved instruction exception on Ingenic XBurst cores.

It would seem to make more sense to use local_flush_icache_range()
instead which does the same thing but is more portable.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h  |  1 -
 arch/mips/kvm/kvm_locore.S        | 32 --------------------------------
 arch/mips/kvm/kvm_mips.c          |  3 ++-
 arch/mips/kvm/kvm_mips_dyntrans.c | 15 +++++++++------
 arch/mips/kvm/kvm_mips_emul.c     |  2 +-
 5 files changed, 12 insertions(+), 41 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 060aaa6348d7..f0e25c6d10b2 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -646,7 +646,6 @@ extern int kvm_mips_trans_mtc0(uint32_t inst, uint32_t *opc,
 			       struct kvm_vcpu *vcpu);
 
 /* Misc */
-extern void mips32_SyncICache(unsigned long addr, unsigned long size);
 extern int kvm_mips_dump_stats(struct kvm_vcpu *vcpu);
 extern unsigned long kvm_mips_get_ramsize(struct kvm *kvm);
 
diff --git a/arch/mips/kvm/kvm_locore.S b/arch/mips/kvm/kvm_locore.S
index bbace092ad0a..033ac343e72c 100644
--- a/arch/mips/kvm/kvm_locore.S
+++ b/arch/mips/kvm/kvm_locore.S
@@ -611,35 +611,3 @@ MIPSX(exceptions):
 	.word _C_LABEL(MIPSX(GuestException))	# 29
 	.word _C_LABEL(MIPSX(GuestException))	# 30
 	.word _C_LABEL(MIPSX(GuestException))	# 31
-
-
-/* This routine makes changes to the instruction stream effective to the hardware.
- * It should be called after the instruction stream is written.
- * On return, the new instructions are effective.
- * Inputs:
- * a0 = Start address of new instruction stream
- * a1 = Size, in bytes, of new instruction stream
- */
-
-#define HW_SYNCI_Step       $1
-LEAF(MIPSX(SyncICache))
-	.set	push
-	.set	mips32r2
-	beq	a1, zero, 20f
-	 nop
-	REG_ADDU a1, a0, a1
-	rdhwr	v0, HW_SYNCI_Step
-	beq	v0, zero, 20f
-	 nop
-10:
-	synci	0(a0)
-	REG_ADDU a0, a0, v0
-	sltu	v1, a0, a1
-	bne	v1, zero, 10b
-	 nop
-	sync
-20:
-	jr.hb	ra
-	 nop
-	.set	pop
-END(MIPSX(SyncICache))
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 5efce56f0df0..14511138f187 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -350,7 +350,8 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	       mips32_GuestExceptionEnd - mips32_GuestException);
 
 	/* Invalidate the icache for these ranges */
-	mips32_SyncICache((unsigned long) gebase, ALIGN(size, PAGE_SIZE));
+	local_flush_icache_range((unsigned long)gebase,
+				(unsigned long)gebase + ALIGN(size, PAGE_SIZE));
 
 	/* Allocate comm page for guest kernel, a TLB will be reserved for mapping GVA @ 0xFFFF8000 to this page */
 	vcpu->arch.kseg0_commpage = kzalloc(PAGE_SIZE << 1, GFP_KERNEL);
diff --git a/arch/mips/kvm/kvm_mips_dyntrans.c b/arch/mips/kvm/kvm_mips_dyntrans.c
index 96528e2d1ea6..b80e41d858fd 100644
--- a/arch/mips/kvm/kvm_mips_dyntrans.c
+++ b/arch/mips/kvm/kvm_mips_dyntrans.c
@@ -16,6 +16,7 @@
 #include <linux/vmalloc.h>
 #include <linux/fs.h>
 #include <linux/bootmem.h>
+#include <asm/cacheflush.h>
 
 #include "kvm_mips_comm.h"
 
@@ -40,7 +41,7 @@ kvm_mips_trans_cache_index(uint32_t inst, uint32_t *opc,
 	    CKSEG0ADDR(kvm_mips_translate_guest_kseg0_to_hpa
 		       (vcpu, (unsigned long) opc));
 	memcpy((void *)kseg0_opc, (void *)&synci_inst, sizeof(uint32_t));
-	mips32_SyncICache(kseg0_opc, 32);
+	local_flush_icache_range(kseg0_opc, kseg0_opc + 32);
 
 	return result;
 }
@@ -66,7 +67,7 @@ kvm_mips_trans_cache_va(uint32_t inst, uint32_t *opc,
 	    CKSEG0ADDR(kvm_mips_translate_guest_kseg0_to_hpa
 		       (vcpu, (unsigned long) opc));
 	memcpy((void *)kseg0_opc, (void *)&synci_inst, sizeof(uint32_t));
-	mips32_SyncICache(kseg0_opc, 32);
+	local_flush_icache_range(kseg0_opc, kseg0_opc + 32);
 
 	return result;
 }
@@ -99,11 +100,12 @@ kvm_mips_trans_mfc0(uint32_t inst, uint32_t *opc, struct kvm_vcpu *vcpu)
 		    CKSEG0ADDR(kvm_mips_translate_guest_kseg0_to_hpa
 			       (vcpu, (unsigned long) opc));
 		memcpy((void *)kseg0_opc, (void *)&mfc0_inst, sizeof(uint32_t));
-		mips32_SyncICache(kseg0_opc, 32);
+		local_flush_icache_range(kseg0_opc, kseg0_opc + 32);
 	} else if (KVM_GUEST_KSEGX((unsigned long) opc) == KVM_GUEST_KSEG23) {
 		local_irq_save(flags);
 		memcpy((void *)opc, (void *)&mfc0_inst, sizeof(uint32_t));
-		mips32_SyncICache((unsigned long) opc, 32);
+		local_flush_icache_range((unsigned long)opc,
+					 (unsigned long)opc + 32);
 		local_irq_restore(flags);
 	} else {
 		kvm_err("%s: Invalid address: %p\n", __func__, opc);
@@ -134,11 +136,12 @@ kvm_mips_trans_mtc0(uint32_t inst, uint32_t *opc, struct kvm_vcpu *vcpu)
 		    CKSEG0ADDR(kvm_mips_translate_guest_kseg0_to_hpa
 			       (vcpu, (unsigned long) opc));
 		memcpy((void *)kseg0_opc, (void *)&mtc0_inst, sizeof(uint32_t));
-		mips32_SyncICache(kseg0_opc, 32);
+		local_flush_icache_range(kseg0_opc, kseg0_opc + 32);
 	} else if (KVM_GUEST_KSEGX((unsigned long) opc) == KVM_GUEST_KSEG23) {
 		local_irq_save(flags);
 		memcpy((void *)opc, (void *)&mtc0_inst, sizeof(uint32_t));
-		mips32_SyncICache((unsigned long) opc, 32);
+		local_flush_icache_range((unsigned long)opc,
+					 (unsigned long)opc + 32);
 		local_irq_restore(flags);
 	} else {
 		kvm_err("%s: Invalid address: %p\n", __func__, opc);
diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index e3fec99941a7..bad31c6235d4 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -887,7 +887,7 @@ int kvm_mips_sync_icache(unsigned long va, struct kvm_vcpu *vcpu)
 
 	printk("%s: va: %#lx, unmapped: %#x\n", __func__, va, CKSEG0ADDR(pa));
 
-	mips32_SyncICache(CKSEG0ADDR(pa), 32);
+	local_flush_icache_range(CKSEG0ADDR(pa), 32);
 	return 0;
 }
 
-- 
1.8.1.2

[PATCH 03/21] MIPS: KVM: Use tlb_write_random

[James Hogan]

When MIPS KVM needs to write a TLB entry for the guest it reads the
CP0_Random register, uses it to generate the CP_Index, and writes the
TLB entry using the TLBWI instruction (tlb_write_indexed()).

However there's an instruction for that, TLBWR (tlb_write_random()) so
use that instead.

This happens to also fix an issue with Ingenic XBurst cores where the
same TLB entry is replaced each time preventing forward progress on
stores due to alternating between TLB load misses for the instruction
fetch and TLB store misses.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_tlb.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 50ab9c4d4a5d..9d371ee0a755 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -222,16 +222,14 @@ kvm_mips_host_tlb_write(struct kvm_vcpu *vcpu, unsigned long entryhi,
 		return -1;
 	}
 
-	if (idx < 0) {
-		idx = read_c0_random() % current_cpu_data.tlbsize;
-		write_c0_index(idx);
-		mtc0_tlbw_hazard();
-	}
 	write_c0_entrylo0(entrylo0);
 	write_c0_entrylo1(entrylo1);
 	mtc0_tlbw_hazard();
 
-	tlb_write_indexed();
+	if (idx < 0)
+		tlb_write_random();
+	else
+		tlb_write_indexed();
 	tlbw_use_hazard();
 
 #ifdef DEBUG
-- 
1.8.1.2

[PATCH 04/21] MIPS: KVM: Fix CP0_EBASE KVM register id

[James Hogan]

KVM_REG_MIPS_CP0_EBASE is defined as 64bit, but is a 32bit register even
in MIPS64, so fix the definition.

Note, this definition isn't actually used yet, so it didn't cause any
problems.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: David Daney <david.daney@cavium.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_mips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 14511138f187..46cea0bad518 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -512,7 +512,7 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 #define KVM_REG_MIPS_CP0_COMPARE	MIPS_CP0_32(11, 0)
 #define KVM_REG_MIPS_CP0_STATUS		MIPS_CP0_32(12, 0)
 #define KVM_REG_MIPS_CP0_CAUSE		MIPS_CP0_32(13, 0)
-#define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_64(15, 1)
+#define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)
 #define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
 #define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
 #define KVM_REG_MIPS_CP0_CONFIG2	MIPS_CP0_32(16, 2)
-- 
1.8.1.2

Re: [PATCH 04/21] MIPS: KVM: Fix CP0_EBASE KVM register id

[David Daney]

On 04/25/2014 08:19 AM, James Hogan wrote:
> KVM_REG_MIPS_CP0_EBASE is defined as 64bit, but is a 32bit register even
> in MIPS64, so fix the definition.
>
> Note, this definition isn't actually used yet, so it didn't cause any
> problems.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: David Daney <david.daney@cavium.com>
> Cc: Sanjay Lal <sanjayl@kymasys.com>
> ---
>   arch/mips/kvm/kvm_mips.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
> index 14511138f187..46cea0bad518 100644
> --- a/arch/mips/kvm/kvm_mips.c
> +++ b/arch/mips/kvm/kvm_mips.c
> @@ -512,7 +512,7 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
>   #define KVM_REG_MIPS_CP0_COMPARE	MIPS_CP0_32(11, 0)
>   #define KVM_REG_MIPS_CP0_STATUS		MIPS_CP0_32(12, 0)
>   #define KVM_REG_MIPS_CP0_CAUSE		MIPS_CP0_32(13, 0)
> -#define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_64(15, 1)
> +#define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)


According to:

  MIPS® Architecture Reference Manual
   Volume III: The MIPS64® and
microMIPS64TM Privileged Resource
Architecture

Document Number: MD00089
Revision 5.02
April 30, 2013

In section 9.39 EBase Register (CP0 Register 15, Select 1), we see that 
EBase can be either 32-bits or 64-bits wide.

I would recommend leaving this as a 64-bit wide register, so that CPU 
implementations with the wider EBase can be supported.

Alternately, probe for the width and use the appropriate 32-bit or 
64-bit to more closely reflect reality.


>   #define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
>   #define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
>   #define KVM_REG_MIPS_CP0_CONFIG2	MIPS_CP0_32(16, 2)
>

Re: [PATCH 04/21] MIPS: KVM: Fix CP0_EBASE KVM register id

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

Hi David,

On Friday 25 April 2014 09:36:50 David Daney wrote:
> On 04/25/2014 08:19 AM, James Hogan wrote:
> > -#define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_64(15, 1)
> > +#define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)
> 
> According to:
> 
>   MIPS® Architecture Reference Manual
>    Volume III: The MIPS64® and
> microMIPS64TM Privileged Resource
> Architecture
> 
> Document Number: MD00089
> Revision 5.02
> April 30, 2013
> 
> In section 9.39 EBase Register (CP0 Register 15, Select 1), we see that
> EBase can be either 32-bits or 64-bits wide.
> 
> I would recommend leaving this as a 64-bit wide register, so that CPU
> implementations with the wider EBase can be supported.

Yes, you're quite right. I should have checked that one for carefully (I did 
think it was a bit odd for it to be 32bit on MIPS64). I'll drop this patch.

Thanks
James

> 
> Alternately, probe for the width and use the appropriate 32-bit or
> 64-bit to more closely reflect reality.
> 
> >   #define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
> >   #define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
> >   #define KVM_REG_MIPS_CP0_CONFIG2	MIPS_CP0_32(16, 2)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 05/21] MIPS: KVM: Add CP0_EPC KVM register access

[James Hogan]

Contrary to the comment, the guest CP0_EPC register cannot be set via
kvm_regs, since it is distinct from the guest PC. Add the EPC register
to the KVM_{GET,SET}_ONE_REG ioctl interface.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: David Daney <david.daney@cavium.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_mips.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 46cea0bad518..db41876cbac5 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -512,6 +512,7 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 #define KVM_REG_MIPS_CP0_COMPARE	MIPS_CP0_32(11, 0)
 #define KVM_REG_MIPS_CP0_STATUS		MIPS_CP0_32(12, 0)
 #define KVM_REG_MIPS_CP0_CAUSE		MIPS_CP0_32(13, 0)
+#define KVM_REG_MIPS_CP0_EPC		MIPS_CP0_64(14, 0)
 #define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)
 #define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
 #define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
@@ -567,7 +568,7 @@ static u64 kvm_mips_get_one_regs[] = {
 	KVM_REG_MIPS_CP0_ENTRYHI,
 	KVM_REG_MIPS_CP0_STATUS,
 	KVM_REG_MIPS_CP0_CAUSE,
-	/* EPC set via kvm_regs, et al. */
+	KVM_REG_MIPS_CP0_EPC,
 	KVM_REG_MIPS_CP0_CONFIG,
 	KVM_REG_MIPS_CP0_CONFIG1,
 	KVM_REG_MIPS_CP0_CONFIG2,
@@ -620,6 +621,9 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_CAUSE:
 		v = (long)kvm_read_c0_guest_cause(cop0);
 		break;
+	case KVM_REG_MIPS_CP0_EPC:
+		v = (long)kvm_read_c0_guest_epc(cop0);
+		break;
 	case KVM_REG_MIPS_CP0_ERROREPC:
 		v = (long)kvm_read_c0_guest_errorepc(cop0);
 		break;
@@ -716,6 +720,9 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_CAUSE:
 		kvm_write_c0_guest_cause(cop0, v);
 		break;
+	case KVM_REG_MIPS_CP0_EPC:
+		kvm_write_c0_guest_epc(cop0, v);
+		break;
 	case KVM_REG_MIPS_CP0_ERROREPC:
 		kvm_write_c0_guest_errorepc(cop0, v);
 		break;
-- 
1.8.1.2

Re: [PATCH 05/21] MIPS: KVM: Add CP0_EPC KVM register access

[David Daney]

On 04/25/2014 08:19 AM, James Hogan wrote:
> Contrary to the comment, the guest CP0_EPC register cannot be set via
> kvm_regs, since it is distinct from the guest PC. Add the EPC register
> to the KVM_{GET,SET}_ONE_REG ioctl interface.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: David Daney <david.daney@cavium.com>
> Cc: Sanjay Lal <sanjayl@kymasys.com>

NACK...


> ---
>   arch/mips/kvm/kvm_mips.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
> index 46cea0bad518..db41876cbac5 100644
> --- a/arch/mips/kvm/kvm_mips.c
> +++ b/arch/mips/kvm/kvm_mips.c
> @@ -512,6 +512,7 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
>   #define KVM_REG_MIPS_CP0_COMPARE	MIPS_CP0_32(11, 0)
>   #define KVM_REG_MIPS_CP0_STATUS		MIPS_CP0_32(12, 0)
>   #define KVM_REG_MIPS_CP0_CAUSE		MIPS_CP0_32(13, 0)
> +#define KVM_REG_MIPS_CP0_EPC		MIPS_CP0_64(14, 0)

This is already called KVM_REG_MIPS_PC, you cannot change that.


>   #define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)
>   #define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
>   #define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
> @@ -567,7 +568,7 @@ static u64 kvm_mips_get_one_regs[] = {
>   	KVM_REG_MIPS_CP0_ENTRYHI,
>   	KVM_REG_MIPS_CP0_STATUS,
>   	KVM_REG_MIPS_CP0_CAUSE,
> -	/* EPC set via kvm_regs, et al. */
> +	KVM_REG_MIPS_CP0_EPC,
>   	KVM_REG_MIPS_CP0_CONFIG,
>   	KVM_REG_MIPS_CP0_CONFIG1,
>   	KVM_REG_MIPS_CP0_CONFIG2,
> @@ -620,6 +621,9 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
>   	case KVM_REG_MIPS_CP0_CAUSE:
>   		v = (long)kvm_read_c0_guest_cause(cop0);
>   		break;
> +	case KVM_REG_MIPS_CP0_EPC:
> +		v = (long)kvm_read_c0_guest_epc(cop0);
> +		break;
>   	case KVM_REG_MIPS_CP0_ERROREPC:
>   		v = (long)kvm_read_c0_guest_errorepc(cop0);
>   		break;
> @@ -716,6 +720,9 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
>   	case KVM_REG_MIPS_CP0_CAUSE:
>   		kvm_write_c0_guest_cause(cop0, v);
>   		break;
> +	case KVM_REG_MIPS_CP0_EPC:
> +		kvm_write_c0_guest_epc(cop0, v);
> +		break;
>   	case KVM_REG_MIPS_CP0_ERROREPC:
>   		kvm_write_c0_guest_errorepc(cop0, v);
>   		break;
>

Re: [PATCH 05/21] MIPS: KVM: Add CP0_EPC KVM register access

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]

Hi David,

On Friday 25 April 2014 09:44:17 David Daney wrote:
> On 04/25/2014 08:19 AM, James Hogan wrote:
> > Contrary to the comment, the guest CP0_EPC register cannot be set via
> > kvm_regs, since it is distinct from the guest PC. Add the EPC register
> > to the KVM_{GET,SET}_ONE_REG ioctl interface.
> > 
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Gleb Natapov <gleb@kernel.org>
> > Cc: kvm@vger.kernel.org
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: linux-mips@linux-mips.org
> > Cc: David Daney <david.daney@cavium.com>
> > Cc: Sanjay Lal <sanjayl@kymasys.com>
> 
> NACK...
> 
> > ---
> > 
> >   arch/mips/kvm/kvm_mips.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
> > index 46cea0bad518..db41876cbac5 100644
> > --- a/arch/mips/kvm/kvm_mips.c
> > +++ b/arch/mips/kvm/kvm_mips.c
> > @@ -512,6 +512,7 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
> > 
> >   #define KVM_REG_MIPS_CP0_COMPARE	MIPS_CP0_32(11, 0)
> >   #define KVM_REG_MIPS_CP0_STATUS		MIPS_CP0_32(12, 0)
> >   #define KVM_REG_MIPS_CP0_CAUSE		MIPS_CP0_32(13, 0)
> > 
> > +#define KVM_REG_MIPS_CP0_EPC		MIPS_CP0_64(14, 0)
> 
> This is already called KVM_REG_MIPS_PC, you cannot change that.

KVM_REG_MIPS_PC gets you vcpu->arch.pc, i.e. the next address of guest 
execution.

KVM_REG_MIPS_CP0_EPC gets you the guest's CP0 EPC register which is the PC of 
the last guest exception.

They are quite distinct state, even though vcpu->arch.pc is read from the 
*root context*'s CP0 EPC register after an exception or interrupt.

Cheers
James

> 
> >   #define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)
> >   #define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
> >   #define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
> > 
> > @@ -567,7 +568,7 @@ static u64 kvm_mips_get_one_regs[] = {
> > 
> >   	KVM_REG_MIPS_CP0_ENTRYHI,
> >   	KVM_REG_MIPS_CP0_STATUS,
> >   	KVM_REG_MIPS_CP0_CAUSE,
> > 
> > -	/* EPC set via kvm_regs, et al. */
> > +	KVM_REG_MIPS_CP0_EPC,
> > 
> >   	KVM_REG_MIPS_CP0_CONFIG,
> >   	KVM_REG_MIPS_CP0_CONFIG1,
> >   	KVM_REG_MIPS_CP0_CONFIG2,
> > 
> > @@ -620,6 +621,9 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
> > 
> >   	case KVM_REG_MIPS_CP0_CAUSE:
> >   		v = (long)kvm_read_c0_guest_cause(cop0);
> >   		break;
> > 
> > +	case KVM_REG_MIPS_CP0_EPC:
> > +		v = (long)kvm_read_c0_guest_epc(cop0);
> > +		break;
> > 
> >   	case KVM_REG_MIPS_CP0_ERROREPC:
> >   		v = (long)kvm_read_c0_guest_errorepc(cop0);
> >   		break;
> > 
> > @@ -716,6 +720,9 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
> > 
> >   	case KVM_REG_MIPS_CP0_CAUSE:
> >   		kvm_write_c0_guest_cause(cop0, v);
> >   		break;
> > 
> > +	case KVM_REG_MIPS_CP0_EPC:
> > +		kvm_write_c0_guest_epc(cop0, v);
> > +		break;
> > 
> >   	case KVM_REG_MIPS_CP0_ERROREPC:
> >   		kvm_write_c0_guest_errorepc(cop0, v);
> >   		break;

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 05/21] MIPS: KVM: Add CP0_EPC KVM register access

[David Daney]

On 04/25/2014 01:29 PM, James Hogan wrote:
> Hi David,
>
> On Friday 25 April 2014 09:44:17 David Daney wrote:
>> On 04/25/2014 08:19 AM, James Hogan wrote:
>>> Contrary to the comment, the guest CP0_EPC register cannot be set via
>>> kvm_regs, since it is distinct from the guest PC. Add the EPC register
>>> to the KVM_{GET,SET}_ONE_REG ioctl interface.
>>>
>>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Gleb Natapov <gleb@kernel.org>
>>> Cc: kvm@vger.kernel.org
>>> Cc: Ralf Baechle <ralf@linux-mips.org>
>>> Cc: linux-mips@linux-mips.org
>>> Cc: David Daney <david.daney@cavium.com>
>>> Cc: Sanjay Lal <sanjayl@kymasys.com>
>>
>> NACK...
>>
>>> ---
>>>
>>>    arch/mips/kvm/kvm_mips.c | 9 ++++++++-
>>>    1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
>>> index 46cea0bad518..db41876cbac5 100644
>>> --- a/arch/mips/kvm/kvm_mips.c
>>> +++ b/arch/mips/kvm/kvm_mips.c
>>> @@ -512,6 +512,7 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
>>>
>>>    #define KVM_REG_MIPS_CP0_COMPARE	MIPS_CP0_32(11, 0)
>>>    #define KVM_REG_MIPS_CP0_STATUS		MIPS_CP0_32(12, 0)
>>>    #define KVM_REG_MIPS_CP0_CAUSE		MIPS_CP0_32(13, 0)
>>>
>>> +#define KVM_REG_MIPS_CP0_EPC		MIPS_CP0_64(14, 0)
>>
>> This is already called KVM_REG_MIPS_PC, you cannot change that.
>
> KVM_REG_MIPS_PC gets you vcpu->arch.pc, i.e. the next address of guest
> execution.
>
> KVM_REG_MIPS_CP0_EPC gets you the guest's CP0 EPC register which is the PC of
> the last guest exception.
>
> They are quite distinct state, even though vcpu->arch.pc is read from the
> *root context*'s CP0 EPC register after an exception or interrupt.
>

Sorry, my mistake.  I was confusing Root/Host and Guest state.

I remove my objection.

David Daney


> Cheers
> James
>
>>
>>>    #define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)
>>>    #define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
>>>    #define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
>>>
>>> @@ -567,7 +568,7 @@ static u64 kvm_mips_get_one_regs[] = {
>>>
>>>    	KVM_REG_MIPS_CP0_ENTRYHI,
>>>    	KVM_REG_MIPS_CP0_STATUS,
>>>    	KVM_REG_MIPS_CP0_CAUSE,
>>>
>>> -	/* EPC set via kvm_regs, et al. */
>>> +	KVM_REG_MIPS_CP0_EPC,
>>>
>>>    	KVM_REG_MIPS_CP0_CONFIG,
>>>    	KVM_REG_MIPS_CP0_CONFIG1,
>>>    	KVM_REG_MIPS_CP0_CONFIG2,
>>>
>>> @@ -620,6 +621,9 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
>>>
>>>    	case KVM_REG_MIPS_CP0_CAUSE:
>>>    		v = (long)kvm_read_c0_guest_cause(cop0);
>>>    		break;
>>>
>>> +	case KVM_REG_MIPS_CP0_EPC:
>>> +		v = (long)kvm_read_c0_guest_epc(cop0);
>>> +		break;
>>>
>>>    	case KVM_REG_MIPS_CP0_ERROREPC:
>>>    		v = (long)kvm_read_c0_guest_errorepc(cop0);
>>>    		break;
>>>
>>> @@ -716,6 +720,9 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
>>>
>>>    	case KVM_REG_MIPS_CP0_CAUSE:
>>>    		kvm_write_c0_guest_cause(cop0, v);
>>>    		break;
>>>
>>> +	case KVM_REG_MIPS_CP0_EPC:
>>> +		kvm_write_c0_guest_epc(cop0, v);
>>> +		break;
>>>
>>>    	case KVM_REG_MIPS_CP0_ERROREPC:
>>>    		kvm_write_c0_guest_errorepc(cop0, v);
>>>    		break;

[PATCH 06/21] MIPS: KVM: Move KVM_{GET,SET}_ONE_REG definitions into kvm_host.h

[James Hogan]

Move the KVM_{GET,SET}_ONE_REG MIPS register id definitions out of
kvm_mips.c to kvm_host.h so that they can be shared between multiple
source files. This allows register access to be indirected depending on
the underlying implementation (trap & emulate or VZ).

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: David Daney <david.daney@cavium.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h | 32 ++++++++++++++++++++++++++++++++
 arch/mips/kvm/kvm_mips.c         | 31 -------------------------------
 2 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index f0e25c6d10b2..0fe6a4c71bba 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -19,6 +19,38 @@
 #include <linux/threads.h>
 #include <linux/spinlock.h>
 
+/* MIPS KVM register ids */
+#define MIPS_CP0_32(_R, _S)					\
+	(KVM_REG_MIPS | KVM_REG_SIZE_U32 | 0x10000 | (8 * (_R) + (_S)))
+
+#define MIPS_CP0_64(_R, _S)					\
+	(KVM_REG_MIPS | KVM_REG_SIZE_U64 | 0x10000 | (8 * (_R) + (_S)))
+
+#define KVM_REG_MIPS_CP0_INDEX		MIPS_CP0_32(0, 0)
+#define KVM_REG_MIPS_CP0_ENTRYLO0	MIPS_CP0_64(2, 0)
+#define KVM_REG_MIPS_CP0_ENTRYLO1	MIPS_CP0_64(3, 0)
+#define KVM_REG_MIPS_CP0_CONTEXT	MIPS_CP0_64(4, 0)
+#define KVM_REG_MIPS_CP0_USERLOCAL	MIPS_CP0_64(4, 2)
+#define KVM_REG_MIPS_CP0_PAGEMASK	MIPS_CP0_32(5, 0)
+#define KVM_REG_MIPS_CP0_PAGEGRAIN	MIPS_CP0_32(5, 1)
+#define KVM_REG_MIPS_CP0_WIRED		MIPS_CP0_32(6, 0)
+#define KVM_REG_MIPS_CP0_HWRENA		MIPS_CP0_32(7, 0)
+#define KVM_REG_MIPS_CP0_BADVADDR	MIPS_CP0_64(8, 0)
+#define KVM_REG_MIPS_CP0_COUNT		MIPS_CP0_32(9, 0)
+#define KVM_REG_MIPS_CP0_ENTRYHI	MIPS_CP0_64(10, 0)
+#define KVM_REG_MIPS_CP0_COMPARE	MIPS_CP0_32(11, 0)
+#define KVM_REG_MIPS_CP0_STATUS		MIPS_CP0_32(12, 0)
+#define KVM_REG_MIPS_CP0_CAUSE		MIPS_CP0_32(13, 0)
+#define KVM_REG_MIPS_CP0_EPC		MIPS_CP0_64(14, 0)
+#define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)
+#define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
+#define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
+#define KVM_REG_MIPS_CP0_CONFIG2	MIPS_CP0_32(16, 2)
+#define KVM_REG_MIPS_CP0_CONFIG3	MIPS_CP0_32(16, 3)
+#define KVM_REG_MIPS_CP0_CONFIG7	MIPS_CP0_32(16, 7)
+#define KVM_REG_MIPS_CP0_XCONTEXT	MIPS_CP0_64(20, 0)
+#define KVM_REG_MIPS_CP0_ERROREPC	MIPS_CP0_64(30, 0)
+
 
 #define KVM_MAX_VCPUS		1
 #define KVM_USER_MEM_SLOTS	8
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index db41876cbac5..a4dd796dfa67 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -491,37 +491,6 @@ kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
 	return -ENOIOCTLCMD;
 }
 
-#define MIPS_CP0_32(_R, _S)					\
-	(KVM_REG_MIPS | KVM_REG_SIZE_U32 | 0x10000 | (8 * (_R) + (_S)))
-
-#define MIPS_CP0_64(_R, _S)					\
-	(KVM_REG_MIPS | KVM_REG_SIZE_U64 | 0x10000 | (8 * (_R) + (_S)))
-
-#define KVM_REG_MIPS_CP0_INDEX		MIPS_CP0_32(0, 0)
-#define KVM_REG_MIPS_CP0_ENTRYLO0	MIPS_CP0_64(2, 0)
-#define KVM_REG_MIPS_CP0_ENTRYLO1	MIPS_CP0_64(3, 0)
-#define KVM_REG_MIPS_CP0_CONTEXT	MIPS_CP0_64(4, 0)
-#define KVM_REG_MIPS_CP0_USERLOCAL	MIPS_CP0_64(4, 2)
-#define KVM_REG_MIPS_CP0_PAGEMASK	MIPS_CP0_32(5, 0)
-#define KVM_REG_MIPS_CP0_PAGEGRAIN	MIPS_CP0_32(5, 1)
-#define KVM_REG_MIPS_CP0_WIRED		MIPS_CP0_32(6, 0)
-#define KVM_REG_MIPS_CP0_HWRENA		MIPS_CP0_32(7, 0)
-#define KVM_REG_MIPS_CP0_BADVADDR	MIPS_CP0_64(8, 0)
-#define KVM_REG_MIPS_CP0_COUNT		MIPS_CP0_32(9, 0)
-#define KVM_REG_MIPS_CP0_ENTRYHI	MIPS_CP0_64(10, 0)
-#define KVM_REG_MIPS_CP0_COMPARE	MIPS_CP0_32(11, 0)
-#define KVM_REG_MIPS_CP0_STATUS		MIPS_CP0_32(12, 0)
-#define KVM_REG_MIPS_CP0_CAUSE		MIPS_CP0_32(13, 0)
-#define KVM_REG_MIPS_CP0_EPC		MIPS_CP0_64(14, 0)
-#define KVM_REG_MIPS_CP0_EBASE		MIPS_CP0_32(15, 1)
-#define KVM_REG_MIPS_CP0_CONFIG		MIPS_CP0_32(16, 0)
-#define KVM_REG_MIPS_CP0_CONFIG1	MIPS_CP0_32(16, 1)
-#define KVM_REG_MIPS_CP0_CONFIG2	MIPS_CP0_32(16, 2)
-#define KVM_REG_MIPS_CP0_CONFIG3	MIPS_CP0_32(16, 3)
-#define KVM_REG_MIPS_CP0_CONFIG7	MIPS_CP0_32(16, 7)
-#define KVM_REG_MIPS_CP0_XCONTEXT	MIPS_CP0_64(20, 0)
-#define KVM_REG_MIPS_CP0_ERROREPC	MIPS_CP0_64(30, 0)
-
 static u64 kvm_mips_get_one_regs[] = {
 	KVM_REG_MIPS_R0,
 	KVM_REG_MIPS_R1,
-- 
1.8.1.2

[PATCH 07/21] MIPS: KVM: Add CP0_Count/Compare KVM register access

[James Hogan]

Implement KVM_{GET,SET}_ONE_REG ioctl based access to the guest CP0
Count and Compare registers. These registers are special in that writing
to them has side effects (adjusting the time until the next timer
interrupt) and reading of Count depends on the time. Therefore add a
couple of callbacks so that different implementations (trap & emulate or
VZ) can implement them differently depending on what the hardware
provides.

The trap & emulate versions mostly duplicate what happens when a T&E
guest reads or writes these registers, so it inherits the same
limitations which can be fixed in later patches.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: David Daney <david.daney@cavium.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h |  4 ++++
 arch/mips/kvm/kvm_mips.c         | 16 ++++++++++++++++
 arch/mips/kvm/kvm_trap_emul.c    | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 0fe6a4c71bba..3eedcb3015e5 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -523,6 +523,10 @@ struct kvm_mips_callbacks {
 			    uint32_t cause);
 	int (*irq_clear) (struct kvm_vcpu *vcpu, unsigned int priority,
 			  uint32_t cause);
+	int (*get_one_reg)(struct kvm_vcpu *vcpu,
+			   const struct kvm_one_reg *reg, s64 *v);
+	int (*set_one_reg)(struct kvm_vcpu *vcpu,
+			   const struct kvm_one_reg *reg, s64 v);
 };
 extern struct kvm_mips_callbacks *kvm_mips_callbacks;
 int kvm_mips_emulation_init(struct kvm_mips_callbacks **install_callbacks);
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index a4dd796dfa67..2f6344cca653 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -534,7 +534,9 @@ static u64 kvm_mips_get_one_regs[] = {
 	KVM_REG_MIPS_CP0_PAGEMASK,
 	KVM_REG_MIPS_CP0_WIRED,
 	KVM_REG_MIPS_CP0_BADVADDR,
+	KVM_REG_MIPS_CP0_COUNT,
 	KVM_REG_MIPS_CP0_ENTRYHI,
+	KVM_REG_MIPS_CP0_COMPARE,
 	KVM_REG_MIPS_CP0_STATUS,
 	KVM_REG_MIPS_CP0_CAUSE,
 	KVM_REG_MIPS_CP0_EPC,
@@ -550,6 +552,7 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 			    const struct kvm_one_reg *reg)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	int ret;
 	s64 v;
 
 	switch (reg->id) {
@@ -584,6 +587,9 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_ENTRYHI:
 		v = (long)kvm_read_c0_guest_entryhi(cop0);
 		break;
+	case KVM_REG_MIPS_CP0_COMPARE:
+		v = (long)kvm_read_c0_guest_compare(cop0);
+		break;
 	case KVM_REG_MIPS_CP0_STATUS:
 		v = (long)kvm_read_c0_guest_status(cop0);
 		break;
@@ -611,6 +617,12 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_CONFIG7:
 		v = (long)kvm_read_c0_guest_config7(cop0);
 		break;
+	/* registers to be handled specially */
+	case KVM_REG_MIPS_CP0_COUNT:
+		ret = kvm_mips_callbacks->get_one_reg(vcpu, reg, &v);
+		if (ret)
+			return ret;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -695,6 +707,10 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_ERROREPC:
 		kvm_write_c0_guest_errorepc(cop0, v);
 		break;
+	/* registers to be handled specially */
+	case KVM_REG_MIPS_CP0_COUNT:
+	case KVM_REG_MIPS_CP0_COMPARE:
+		return kvm_mips_callbacks->set_one_reg(vcpu, reg, v);
 	default:
 		return -EINVAL;
 	}
diff --git a/arch/mips/kvm/kvm_trap_emul.c b/arch/mips/kvm/kvm_trap_emul.c
index 30d725321db1..f1e8389f8d33 100644
--- a/arch/mips/kvm/kvm_trap_emul.c
+++ b/arch/mips/kvm/kvm_trap_emul.c
@@ -401,6 +401,40 @@ static int kvm_trap_emul_vcpu_setup(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
+static int kvm_trap_emul_get_one_reg(struct kvm_vcpu *vcpu,
+				     const struct kvm_one_reg *reg,
+				     s64 *v)
+{
+	switch (reg->id) {
+	case KVM_REG_MIPS_CP0_COUNT:
+		/* XXXKYMA: Run the Guest count register @ 1/4 the rate of the host */
+		*v = (read_c0_count() >> 2);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu,
+				     const struct kvm_one_reg *reg,
+				     s64 v)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+
+	switch (reg->id) {
+	case KVM_REG_MIPS_CP0_COUNT:
+		/* Not supported yet */
+		break;
+	case KVM_REG_MIPS_CP0_COMPARE:
+		kvm_write_c0_guest_compare(cop0, v);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static struct kvm_mips_callbacks kvm_trap_emul_callbacks = {
 	/* exit handlers */
 	.handle_cop_unusable = kvm_trap_emul_handle_cop_unusable,
@@ -423,6 +457,8 @@ static struct kvm_mips_callbacks kvm_trap_emul_callbacks = {
 	.dequeue_io_int = kvm_mips_dequeue_io_int_cb,
 	.irq_deliver = kvm_mips_irq_deliver_cb,
 	.irq_clear = kvm_mips_irq_clear_cb,
+	.get_one_reg = kvm_trap_emul_get_one_reg,
+	.set_one_reg = kvm_trap_emul_set_one_reg,
 };
 
 int kvm_mips_emulation_init(struct kvm_mips_callbacks **install_callbacks)
-- 
1.8.1.2

[PATCH 08/21] MIPS: KVM: Deliver guest interrupts after local_irq_disable()

[James Hogan]

When about to run the guest, deliver guest interrupts after disabling
host interrupts. This should prevent an hrtimer interrupt from being
handled after delivering guest interrupts, and therefore not delivering
the guest timer interrupt until after the next guest exit.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_mips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 2f6344cca653..f6499ccefd9d 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -424,11 +424,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 		vcpu->mmio_needed = 0;
 	}
 
+	local_irq_disable();
 	/* Check if we have any exceptions/interrupts pending */
 	kvm_mips_deliver_interrupts(vcpu,
 				    kvm_read_c0_guest_cause(vcpu->arch.cop0));
 
-	local_irq_disable();
 	kvm_guest_enter();
 
 	r = __kvm_mips_vcpu_run(run, vcpu);
-- 
1.8.1.2

[PATCH 09/21] MIPS: KVM: Fix timer race modifying guest CP0_Cause

[James Hogan]

The hrtimer callback for guest timer timeouts sets the guest's
CP0_Cause.TI bit to indicate to the guest that a timer interrupt is
pending, however there is no mutual exclusion implemented to prevent
this occurring while the guest's CP0_Cause register is being
read-modify-written elsewhere.

When this occurs the setting of the CP0_Cause.TI bit is undone and the
guest misses the timer interrupt and doesn't reprogram the CP0_Compare
register for the next timeout. Currently another timer interrupt will be
triggered again in another 10ms anyway due to the way timers are
emulated, but after the MIPS timer emulation is fixed this would result
in Linux guest time standing still and the guest scheduler not being
invoked until the guest CP0_Count has looped around again, which at
100MHz takes just under 43 seconds.

Currently this is the only asynchronous modification of guest registers,
therefore it is fixed by adjusting the implementations of the
kvm_set_c0_guest_cause(), kvm_clear_c0_guest_cause(), and
kvm_change_c0_guest_cause() macros which are used for modifying the
guest CP0_Cause register to use ll/sc to ensure atomic modification.
This should work in both UP and SMP cases without requiring interrupts
to be disabled.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h | 71 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 6 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 3eedcb3015e5..90e1c0005ff4 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -481,15 +481,74 @@ struct kvm_vcpu_arch {
 #define kvm_read_c0_guest_errorepc(cop0)	(cop0->reg[MIPS_CP0_ERROR_PC][0])
 #define kvm_write_c0_guest_errorepc(cop0, val)	(cop0->reg[MIPS_CP0_ERROR_PC][0] = (val))
 
+/*
+ * Some of the guest registers may be modified asynchronously (e.g. from a
+ * hrtimer callback in hard irq context) and therefore need stronger atomicity
+ * guarantees than other registers.
+ */
+
+static inline void _kvm_atomic_set_c0_guest_reg(unsigned long *reg,
+						unsigned long val)
+{
+	unsigned long temp;
+	do {
+		__asm__ __volatile__(
+		"	.set	mips3				\n"
+		"	" __LL "%0, %1				\n"
+		"	or	%0, %2				\n"
+		"	" __SC	"%0, %1				\n"
+		"	.set	mips0				\n"
+		: "=&r" (temp), "+m" (*reg)
+		: "r" (val));
+	} while (unlikely(!temp));
+}
+
+static inline void _kvm_atomic_clear_c0_guest_reg(unsigned long *reg,
+						  unsigned long val)
+{
+	unsigned long temp;
+	do {
+		__asm__ __volatile__(
+		"	.set	mips3				\n"
+		"	" __LL "%0, %1				\n"
+		"	and	%0, %2				\n"
+		"	" __SC	"%0, %1				\n"
+		"	.set	mips0				\n"
+		: "=&r" (temp), "+m" (*reg)
+		: "r" (~val));
+	} while (unlikely(!temp));
+}
+
+static inline void _kvm_atomic_change_c0_guest_reg(unsigned long *reg,
+						   unsigned long change,
+						   unsigned long val)
+{
+	unsigned long temp;
+	do {
+		__asm__ __volatile__(
+		"	.set	mips3				\n"
+		"	" __LL "%0, %1				\n"
+		"	and	%0, %2				\n"
+		"	or	%0, %3				\n"
+		"	" __SC	"%0, %1				\n"
+		"	.set	mips0				\n"
+		: "=&r" (temp), "+m" (*reg)
+		: "r" (~change), "r" (val & change));
+	} while (unlikely(!temp));
+}
+
 #define kvm_set_c0_guest_status(cop0, val)	(cop0->reg[MIPS_CP0_STATUS][0] |= (val))
 #define kvm_clear_c0_guest_status(cop0, val)	(cop0->reg[MIPS_CP0_STATUS][0] &= ~(val))
-#define kvm_set_c0_guest_cause(cop0, val)	(cop0->reg[MIPS_CP0_CAUSE][0] |= (val))
-#define kvm_clear_c0_guest_cause(cop0, val)	(cop0->reg[MIPS_CP0_CAUSE][0] &= ~(val))
+
+/* Cause can be modified asynchronously from hardirq hrtimer callback */
+#define kvm_set_c0_guest_cause(cop0, val)				\
+	_kvm_atomic_set_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0], val)
+#define kvm_clear_c0_guest_cause(cop0, val)				\
+	_kvm_atomic_clear_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0], val)
 #define kvm_change_c0_guest_cause(cop0, change, val)			\
-{									\
-	kvm_clear_c0_guest_cause(cop0, change);				\
-	kvm_set_c0_guest_cause(cop0, ((val) & (change)));		\
-}
+	_kvm_atomic_change_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0],	\
+					change, val)
+
 #define kvm_set_c0_guest_ebase(cop0, val)	(cop0->reg[MIPS_CP0_PRID][1] |= (val))
 #define kvm_clear_c0_guest_ebase(cop0, val)	(cop0->reg[MIPS_CP0_PRID][1] &= ~(val))
 #define kvm_change_c0_guest_ebase(cop0, change, val)			\
-- 
1.8.1.2

Re: [PATCH 09/21] MIPS: KVM: Fix timer race modifying guest CP0_Cause

[David Daney]

On 04/25/2014 08:19 AM, James Hogan wrote:
> The hrtimer callback for guest timer timeouts sets the guest's
> CP0_Cause.TI bit to indicate to the guest that a timer interrupt is
> pending, however there is no mutual exclusion implemented to prevent
> this occurring while the guest's CP0_Cause register is being
> read-modify-written elsewhere.
>
> When this occurs the setting of the CP0_Cause.TI bit is undone and the
> guest misses the timer interrupt and doesn't reprogram the CP0_Compare
> register for the next timeout. Currently another timer interrupt will be
> triggered again in another 10ms anyway due to the way timers are
> emulated, but after the MIPS timer emulation is fixed this would result
> in Linux guest time standing still and the guest scheduler not being
> invoked until the guest CP0_Count has looped around again, which at
> 100MHz takes just under 43 seconds.
>
> Currently this is the only asynchronous modification of guest registers,
> therefore it is fixed by adjusting the implementations of the
> kvm_set_c0_guest_cause(), kvm_clear_c0_guest_cause(), and
> kvm_change_c0_guest_cause() macros which are used for modifying the
> guest CP0_Cause register to use ll/sc to ensure atomic modification.
> This should work in both UP and SMP cases without requiring interrupts
> to be disabled.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: Sanjay Lal <sanjayl@kymasys.com>

NACK, I don't like the names of these functions.  They initially 
confused me too much...

> ---
>   arch/mips/include/asm/kvm_host.h | 71 ++++++++++++++++++++++++++++++++++++----
>   1 file changed, 65 insertions(+), 6 deletions(-)
>
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index 3eedcb3015e5..90e1c0005ff4 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -481,15 +481,74 @@ struct kvm_vcpu_arch {
>   #define kvm_read_c0_guest_errorepc(cop0)	(cop0->reg[MIPS_CP0_ERROR_PC][0])
>   #define kvm_write_c0_guest_errorepc(cop0, val)	(cop0->reg[MIPS_CP0_ERROR_PC][0] = (val))
>
> +/*
> + * Some of the guest registers may be modified asynchronously (e.g. from a
> + * hrtimer callback in hard irq context) and therefore need stronger atomicity
> + * guarantees than other registers.
> + */
> +
> +static inline void _kvm_atomic_set_c0_guest_reg(unsigned long *reg,
> +						unsigned long val)

The name of this function is too unclear.

It should be _kvm_atomic_or_c0_guest_reg, or 
_kvm_atomic_setbits_c0_guest_reg(unsigned long *reg, unsigned long mask)

> +{
> +	unsigned long temp;
> +	do {
> +		__asm__ __volatile__(
> +		"	.set	mips3				\n"
> +		"	" __LL "%0, %1				\n"
> +		"	or	%0, %2				\n"
> +		"	" __SC	"%0, %1				\n"
> +		"	.set	mips0				\n"
> +		: "=&r" (temp), "+m" (*reg)
> +		: "r" (val));
> +	} while (unlikely(!temp));
> +}
> +
> +static inline void _kvm_atomic_clear_c0_guest_reg(unsigned long *reg,
> +						  unsigned long val)

Same here.

Perhaps _kvm_atomic_clearbits_c0_guest_reg(unsigned long *reg, unsigned 
long mask)

> +{
> +	unsigned long temp;
> +	do {
> +		__asm__ __volatile__(
> +		"	.set	mips3				\n"
> +		"	" __LL "%0, %1				\n"
> +		"	and	%0, %2				\n"
> +		"	" __SC	"%0, %1				\n"
> +		"	.set	mips0				\n"
> +		: "=&r" (temp), "+m" (*reg)
> +		: "r" (~val));
> +	} while (unlikely(!temp));
> +}
> +
> +static inline void _kvm_atomic_change_c0_guest_reg(unsigned long *reg,
> +						   unsigned long change,
> +						   unsigned long val)

Same here.

Perhaps

_kvm_atomic_setbits_c0_guest_reg(unsigned long *reg,
				unsigned long mask,
				unsigned long bits)

> +{
> +	unsigned long temp;
> +	do {
> +		__asm__ __volatile__(
> +		"	.set	mips3				\n"
> +		"	" __LL "%0, %1				\n"
> +		"	and	%0, %2				\n"
> +		"	or	%0, %3				\n"
> +		"	" __SC	"%0, %1				\n"
> +		"	.set	mips0				\n"
> +		: "=&r" (temp), "+m" (*reg)
> +		: "r" (~change), "r" (val & change));
> +	} while (unlikely(!temp));
> +}
> +
>   #define kvm_set_c0_guest_status(cop0, val)	(cop0->reg[MIPS_CP0_STATUS][0] |= (val))
>   #define kvm_clear_c0_guest_status(cop0, val)	(cop0->reg[MIPS_CP0_STATUS][0] &= ~(val))
> -#define kvm_set_c0_guest_cause(cop0, val)	(cop0->reg[MIPS_CP0_CAUSE][0] |= (val))
> -#define kvm_clear_c0_guest_cause(cop0, val)	(cop0->reg[MIPS_CP0_CAUSE][0] &= ~(val))
> +
> +/* Cause can be modified asynchronously from hardirq hrtimer callback */
> +#define kvm_set_c0_guest_cause(cop0, val)				\
> +	_kvm_atomic_set_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0], val)
> +#define kvm_clear_c0_guest_cause(cop0, val)				\
> +	_kvm_atomic_clear_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0], val)
>   #define kvm_change_c0_guest_cause(cop0, change, val)			\
> -{									\
> -	kvm_clear_c0_guest_cause(cop0, change);				\
> -	kvm_set_c0_guest_cause(cop0, ((val) & (change)));		\
> -}
> +	_kvm_atomic_change_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0],	\
> +					change, val)
> +
>   #define kvm_set_c0_guest_ebase(cop0, val)	(cop0->reg[MIPS_CP0_PRID][1] |= (val))
>   #define kvm_clear_c0_guest_ebase(cop0, val)	(cop0->reg[MIPS_CP0_PRID][1] &= ~(val))
>   #define kvm_change_c0_guest_ebase(cop0, change, val)			\
>

Re: [PATCH 09/21] MIPS: KVM: Fix timer race modifying guest CP0_Cause

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 5983 bytes --]

Hi David,

On Friday 25 April 2014 09:55:47 David Daney wrote:
> On 04/25/2014 08:19 AM, James Hogan wrote:
> > The hrtimer callback for guest timer timeouts sets the guest's
> > CP0_Cause.TI bit to indicate to the guest that a timer interrupt is
> > pending, however there is no mutual exclusion implemented to prevent
> > this occurring while the guest's CP0_Cause register is being
> > read-modify-written elsewhere.
> > 
> > When this occurs the setting of the CP0_Cause.TI bit is undone and the
> > guest misses the timer interrupt and doesn't reprogram the CP0_Compare
> > register for the next timeout. Currently another timer interrupt will be
> > triggered again in another 10ms anyway due to the way timers are
> > emulated, but after the MIPS timer emulation is fixed this would result
> > in Linux guest time standing still and the guest scheduler not being
> > invoked until the guest CP0_Count has looped around again, which at
> > 100MHz takes just under 43 seconds.
> > 
> > Currently this is the only asynchronous modification of guest registers,
> > therefore it is fixed by adjusting the implementations of the
> > kvm_set_c0_guest_cause(), kvm_clear_c0_guest_cause(), and
> > kvm_change_c0_guest_cause() macros which are used for modifying the
> > guest CP0_Cause register to use ll/sc to ensure atomic modification.
> > This should work in both UP and SMP cases without requiring interrupts
> > to be disabled.
> > 
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Gleb Natapov <gleb@kernel.org>
> > Cc: kvm@vger.kernel.org
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: linux-mips@linux-mips.org
> > Cc: Sanjay Lal <sanjayl@kymasys.com>
> 
> NACK, I don't like the names of these functions.  They initially
> confused me too much...

It's just being consistent with all the other *set*, *clear*, and *change* 
macros in kvm_host.h, asm/mipsregs.h, and the 229 users of those macros across 
the arch. I see no reason to confuse things further by being inconsistent.

Cheers
James

> 
> > ---
> > 
> >   arch/mips/include/asm/kvm_host.h | 71
> >   ++++++++++++++++++++++++++++++++++++---- 1 file changed, 65
> >   insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/mips/include/asm/kvm_host.h
> > b/arch/mips/include/asm/kvm_host.h index 3eedcb3015e5..90e1c0005ff4
> > 100644
> > --- a/arch/mips/include/asm/kvm_host.h
> > +++ b/arch/mips/include/asm/kvm_host.h
> > @@ -481,15 +481,74 @@ struct kvm_vcpu_arch {
> > 
> >   #define
> >   kvm_read_c0_guest_errorepc(cop0)	(cop0->reg[MIPS_CP0_ERROR_PC][0])
> >   #define kvm_write_c0_guest_errorepc(cop0,
> >   val)	(cop0->reg[MIPS_CP0_ERROR_PC][0] = (val))> 
> > +/*
> > + * Some of the guest registers may be modified asynchronously (e.g. from
> > a
> > + * hrtimer callback in hard irq context) and therefore need stronger
> > atomicity + * guarantees than other registers.
> > + */
> > +
> > +static inline void _kvm_atomic_set_c0_guest_reg(unsigned long *reg,
> > +						unsigned long val)
> 
> The name of this function is too unclear.
> 
> It should be _kvm_atomic_or_c0_guest_reg, or
> _kvm_atomic_setbits_c0_guest_reg(unsigned long *reg, unsigned long mask)
> 
> > +{
> > +	unsigned long temp;
> > +	do {
> > +		__asm__ __volatile__(
> > +		"	.set	mips3				\n"
> > +		"	" __LL "%0, %1				\n"
> > +		"	or	%0, %2				\n"
> > +		"	" __SC	"%0, %1				\n"
> > +		"	.set	mips0				\n"
> > +		: "=&r" (temp), "+m" (*reg)
> > +		: "r" (val));
> > +	} while (unlikely(!temp));
> > +}
> > +
> > +static inline void _kvm_atomic_clear_c0_guest_reg(unsigned long *reg,
> > +						  unsigned long val)
> 
> Same here.
> 
> Perhaps _kvm_atomic_clearbits_c0_guest_reg(unsigned long *reg, unsigned
> long mask)
> 
> > +{
> > +	unsigned long temp;
> > +	do {
> > +		__asm__ __volatile__(
> > +		"	.set	mips3				\n"
> > +		"	" __LL "%0, %1				\n"
> > +		"	and	%0, %2				\n"
> > +		"	" __SC	"%0, %1				\n"
> > +		"	.set	mips0				\n"
> > +		: "=&r" (temp), "+m" (*reg)
> > +		: "r" (~val));
> > +	} while (unlikely(!temp));
> > +}
> > +
> > +static inline void _kvm_atomic_change_c0_guest_reg(unsigned long *reg,
> > +						   unsigned long change,
> > +						   unsigned long val)
> 
> Same here.
> 
> Perhaps
> 
> _kvm_atomic_setbits_c0_guest_reg(unsigned long *reg,
> 				unsigned long mask,
> 				unsigned long bits)
> 
> > +{
> > +	unsigned long temp;
> > +	do {
> > +		__asm__ __volatile__(
> > +		"	.set	mips3				\n"
> > +		"	" __LL "%0, %1				\n"
> > +		"	and	%0, %2				\n"
> > +		"	or	%0, %3				\n"
> > +		"	" __SC	"%0, %1				\n"
> > +		"	.set	mips0				\n"
> > +		: "=&r" (temp), "+m" (*reg)
> > +		: "r" (~change), "r" (val & change));
> > +	} while (unlikely(!temp));
> > +}
> > +
> > 
> >   #define kvm_set_c0_guest_status(cop0,
> >   val)	(cop0->reg[MIPS_CP0_STATUS][0] |= (val)) #define
> >   kvm_clear_c0_guest_status(cop0, val)	(cop0->reg[MIPS_CP0_STATUS][0] &=
> >   ~(val))> 
> > -#define kvm_set_c0_guest_cause(cop0, val)	(cop0->reg[MIPS_CP0_CAUSE][0]
> > |= (val)) -#define kvm_clear_c0_guest_cause(cop0,
> > val)	(cop0->reg[MIPS_CP0_CAUSE][0] &= ~(val)) +
> > +/* Cause can be modified asynchronously from hardirq hrtimer callback */
> > +#define kvm_set_c0_guest_cause(cop0, val)				\
> > +	_kvm_atomic_set_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0], val)
> > +#define kvm_clear_c0_guest_cause(cop0, val)				\
> > +	_kvm_atomic_clear_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0], val)
> > 
> >   #define kvm_change_c0_guest_cause(cop0, change, val)			\
> > 
> > -{									\
> > -	kvm_clear_c0_guest_cause(cop0, change);				\
> > -	kvm_set_c0_guest_cause(cop0, ((val) & (change)));		\
> > -}
> > +	_kvm_atomic_change_c0_guest_reg(&cop0->reg[MIPS_CP0_CAUSE][0],	\
> > +					change, val)
> > +
> > 
> >   #define kvm_set_c0_guest_ebase(cop0, val)	(cop0->reg[MIPS_CP0_PRID][1]
> >   |= (val)) #define kvm_clear_c0_guest_ebase(cop0,
> >   val)	(cop0->reg[MIPS_CP0_PRID][1] &= ~(val)) #define
> >   kvm_change_c0_guest_ebase(cop0, change, val)			\

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 10/21] MIPS: KVM: Migrate hrtimer to follow VCPU

[James Hogan]

When a VCPU is scheduled in on a different CPU, refresh the hrtimer used
for emulating count/compare so that it gets migrated to the same CPU.

This should prevent a timer interrupt occurring on a different CPU to
where the guest it relates to is running, which would cause the guest
timer interrupt not to be delivered until after the next guest exit.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h |  1 +
 arch/mips/kvm/kvm_mips_emul.c    | 17 +++++++++++++++++
 arch/mips/kvm/kvm_tlb.c          |  6 ++++++
 3 files changed, 24 insertions(+)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 90e1c0005ff4..f56bb699506e 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -705,6 +705,7 @@ extern enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
 							 struct kvm_run *run);
 
 enum emulation_result kvm_mips_emulate_count(struct kvm_vcpu *vcpu);
+void kvm_mips_migrate_count(struct kvm_vcpu *vcpu);
 
 enum emulation_result kvm_mips_check_privilege(unsigned long cause,
 					       uint32_t *opc,
diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index bad31c6235d4..7be7317be45d 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -249,6 +249,23 @@ enum emulation_result kvm_mips_emulate_count(struct kvm_vcpu *vcpu)
 	return er;
 }
 
+/**
+ * kvm_mips_migrate_count() - Migrate timer.
+ * @vcpu:	Virtual CPU.
+ *
+ * Migrate CP0_Count hrtimer to the current CPU by cancelling and restarting it
+ * if it was running prior to being cancelled.
+ *
+ * Must be called when the VCPU is migrated to a different CPU to ensure that
+ * timer expiry during guest execution interrupts the guest and causes the
+ * interrupt to be delivered in a timely manner.
+ */
+void kvm_mips_migrate_count(struct kvm_vcpu *vcpu)
+{
+	if (hrtimer_cancel(&vcpu->arch.comparecount_timer))
+		hrtimer_restart(&vcpu->arch.comparecount_timer);
+}
+
 enum emulation_result kvm_mips_emul_eret(struct kvm_vcpu *vcpu)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 9d371ee0a755..1f61fe6739da 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -691,6 +691,12 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	if (vcpu->arch.last_sched_cpu != cpu) {
 		kvm_info("[%d->%d]KVM VCPU[%d] switch\n",
 			 vcpu->arch.last_sched_cpu, cpu, vcpu->vcpu_id);
+		/*
+		 * Migrate the timer interrupt to the current CPU so that it
+		 * always interrupts the guest and synchronously triggers a
+		 * guest timer interrupt.
+		 */
+		kvm_mips_migrate_count(vcpu);
 	}
 
 	if (!newasid) {
-- 
1.8.1.2

[PATCH 11/21] MIPS: KVM: Rewrite count/compare timer emulation

[James Hogan]

Previously the emulation of the CPU timer was just enough to get a Linux
guest running but some shortcuts were taken:
 - The guest timer interrupt was hard coded to always happen every 10 ms
   rather than being timed to when CP0_Count would match CP0_Compare.
 - The guest's CP0_Count register was based on the host's CP0_Count
   register. This isn't very portable and fails on cores without a
   CP_Count register implemented such as Ingenic XBurst. It also meant
   that the guest's CP0_Cause.DC bit to disable the CP0_Count register
   took no effect.
 - The guest's CP0_Count register was emulated by just dividing the
   host's CP0_Count register by 4. This resulted in continuity problems
   when used as a clock source, since when the host CP0_Count overflows
   from 0x7fffffff to 0x80000000, the guest CP0_Count transitions
   discontinuously from 0x1fffffff to 0xe0000000.

Therefore rewrite & fix emulation of the guest timer based on the
monotonic kernel time (i.e. ktime_get()). Internally a 32-bit count_bias
value is added to the frequency scaled nanosecond monotonic time to get
the guest's CP0_Count. The frequency of the timer is initialised to
100MHz and cannot yet be changed, but a later patch will allow the
frequency to be configured via the KVM_{GET,SET}_ONE_REG ioctl
interface.

The timer can now be stopped via the CP0_Cause.DC bit (by the guest or
via the KVM_SET_ONE_REG ioctl interface), at which point the current
CP0_Count is stored and can be read directly. When it is restarted the
bias is recalculated such that the CP0_Count value is continuous.

Due to the nature of hrtimer interrupts any read of the guest's
CP0_Count register while it is running triggers a check for whether the
hrtimer has expired, so that the guest/userland cannot observe the
CP0_Count passing CP0_Compare without queuing a timer interrupt. This is
also taken advantage of when stopping the timer to ensure that a pending
timer interrupt is queued.

This replaces the implementation of:
 - Guest read of CP0_Count
 - Guest write of CP0_Count
 - Guest write of CP0_Compare
 - Guest write of CP0_Cause
 - Guest read of HWR 2 (CC) with RDHWR
 - Host read of CP0_Count via KVM_GET_ONE_REG ioctl interface
 - Host write of CP0_Count via KVM_SET_ONE_REG ioctl interface
 - Host write of CP0_Compare via KVM_SET_ONE_REG ioctl interface
 - Host write of CP0_Cause via KVM_SET_ONE_REG ioctl interface

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h |  21 ++-
 arch/mips/kvm/kvm_mips.c         |  10 +-
 arch/mips/kvm/kvm_mips_emul.c    | 395 ++++++++++++++++++++++++++++++++++++---
 arch/mips/kvm/kvm_trap_emul.c    |  27 ++-
 4 files changed, 415 insertions(+), 38 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index f56bb699506e..57c1085fd6ab 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -404,8 +404,15 @@ struct kvm_vcpu_arch {
 
 	u32 io_gpr;		/* GPR used as IO source/target */
 
-	/* Used to calibrate the virutal count register for the guest */
-	int32_t host_cp0_count;
+	struct hrtimer comparecount_timer;
+	/* Count bias from the raw time */
+	uint32_t count_bias;
+	/* Frequency of timer in Hz */
+	uint32_t count_hz;
+	/* Dynamic nanosecond bias (multiple of count_period) to avoid overflow */
+	s64 count_dyn_bias;
+	/* Period of timer tick in ns */
+	u64 count_period;
 
 	/* Bitmask of exceptions that are pending */
 	unsigned long pending_exceptions;
@@ -426,8 +433,6 @@ struct kvm_vcpu_arch {
 	uint32_t guest_kernel_asid[NR_CPUS];
 	struct mm_struct guest_kernel_mm, guest_user_mm;
 
-	struct hrtimer comparecount_timer;
-
 	int last_sched_cpu;
 
 	/* WAIT executed */
@@ -704,8 +709,14 @@ extern enum emulation_result kvm_mips_emulate_bp_exc(unsigned long cause,
 extern enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
 							 struct kvm_run *run);
 
-enum emulation_result kvm_mips_emulate_count(struct kvm_vcpu *vcpu);
+uint32_t kvm_mips_read_count(struct kvm_vcpu *vcpu);
+void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count);
+void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare);
+void kvm_mips_init_count(struct kvm_vcpu *vcpu);
 void kvm_mips_migrate_count(struct kvm_vcpu *vcpu);
+void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu);
+void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu);
+enum hrtimer_restart kvm_mips_count_timeout(struct kvm_vcpu *vcpu);
 
 enum emulation_result kvm_mips_check_privilege(unsigned long cause,
 					       uint32_t *opc,
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index f6499ccefd9d..b32e05efe8b7 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -368,7 +368,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	vcpu->arch.last_sched_cpu = -1;
 
 	/* Start off the timer */
-	kvm_mips_emulate_count(vcpu);
+	kvm_mips_init_count(vcpu);
 
 	return vcpu;
 
@@ -698,9 +698,6 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_STATUS:
 		kvm_write_c0_guest_status(cop0, v);
 		break;
-	case KVM_REG_MIPS_CP0_CAUSE:
-		kvm_write_c0_guest_cause(cop0, v);
-		break;
 	case KVM_REG_MIPS_CP0_EPC:
 		kvm_write_c0_guest_epc(cop0, v);
 		break;
@@ -710,6 +707,7 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	/* registers to be handled specially */
 	case KVM_REG_MIPS_CP0_COUNT:
 	case KVM_REG_MIPS_CP0_COMPARE:
+	case KVM_REG_MIPS_CP0_CAUSE:
 		return kvm_mips_callbacks->set_one_reg(vcpu, reg, v);
 	default:
 		return -EINVAL;
@@ -983,9 +981,7 @@ enum hrtimer_restart kvm_mips_comparecount_wakeup(struct hrtimer *timer)
 
 	vcpu = container_of(timer, struct kvm_vcpu, arch.comparecount_timer);
 	kvm_mips_comparecount_func((unsigned long) vcpu);
-	hrtimer_forward_now(&vcpu->arch.comparecount_timer,
-			    ktime_set(0, MS_TO_NS(10)));
-	return HRTIMER_RESTART;
+	return kvm_mips_count_timeout(vcpu);
 }
 
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index 7be7317be45d..8ebfd675a3a1 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -11,6 +11,7 @@
 
 #include <linux/errno.h>
 #include <linux/err.h>
+#include <linux/ktime.h>
 #include <linux/kvm_host.h>
 #include <linux/module.h>
 #include <linux/vmalloc.h>
@@ -228,25 +229,278 @@ enum emulation_result update_pc(struct kvm_vcpu *vcpu, uint32_t cause)
 	return er;
 }
 
-/* Everytime the compare register is written to, we need to decide when to fire
- * the timer that represents timer ticks to the GUEST.
+/**
+ * kvm_mips_count_disabled() - Find whether the CP0_Count timer is disabled.
+ * @vcpu:	Virtual CPU.
  *
+ * Returns:	1 if the CP0_Count timer is disabled by the guest CP0_Cause.DC
+ *		bit.
+ *		0 otherwise (in which case CP0_Count timer is running).
  */
-enum emulation_result kvm_mips_emulate_count(struct kvm_vcpu *vcpu)
+static inline int kvm_mips_count_disabled(struct kvm_vcpu *vcpu)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
-	enum emulation_result er = EMULATE_DONE;
+	return kvm_read_c0_guest_cause(cop0) & CAUSEF_DC;
+}
 
-	/* If COUNT is enabled */
-	if (!(kvm_read_c0_guest_cause(cop0) & CAUSEF_DC)) {
-		hrtimer_try_to_cancel(&vcpu->arch.comparecount_timer);
-		hrtimer_start(&vcpu->arch.comparecount_timer,
-			      ktime_set(0, MS_TO_NS(10)), HRTIMER_MODE_REL);
-	} else {
-		hrtimer_try_to_cancel(&vcpu->arch.comparecount_timer);
+/**
+ * kvm_mips_ktime_to_count() - Scale ktime_t to a 32-bit count.
+ *
+ * Caches the dynamic nanosecond bias in vcpu->arch.count_dyn_bias.
+ *
+ * Assumes !kvm_mips_count_disabled(@vcpu) (guest CP0_Count timer is running).
+ */
+static uint32_t kvm_mips_ktime_to_count(struct kvm_vcpu *vcpu, ktime_t now)
+{
+	s64 now_ns, periods;
+	u64 delta;
+
+	now_ns = ktime_to_ns(now);
+	delta = now_ns + vcpu->arch.count_dyn_bias;
+
+	if (delta >= vcpu->arch.count_period) {
+		/* If delta is out of safe range the bias needs adjusting */
+		periods = div64_s64(now_ns, vcpu->arch.count_period);
+		vcpu->arch.count_dyn_bias = -periods * vcpu->arch.count_period;
+		/* Recalculate delta with new bias */
+		delta = now_ns + vcpu->arch.count_dyn_bias;
 	}
 
-	return er;
+	/*
+	 * We've ensured that:
+	 *   delta < count_period
+	 *
+	 * Therefore the intermediate delta*count_hz will never overflow since
+	 * at the boundary condition:
+	 *   delta = count_period
+	 *   delta = NSEC_PER_SEC * 2^32 / count_hz
+	 *   delta * count_hz = NSEC_PER_SEC * 2^32
+	 */
+	return div_u64(delta * vcpu->arch.count_hz, NSEC_PER_SEC);
+}
+
+/**
+ * kvm_mips_read_count_running() - Read the current count value as if running.
+ * @vcpu:	Virtual CPU.
+ * @now:	Kernel time to read CP0_Count at.
+ *
+ * Returns the current guest CP0_Count register at time @now and handles if the
+ * timer interrupt is pending and hasn't been handled yet.
+ *
+ * Returns:	The current value of the guest CP0_Count register.
+ */
+static uint32_t kvm_mips_read_count_running(struct kvm_vcpu *vcpu, ktime_t now)
+{
+	ktime_t expires;
+	int running;
+
+	/* Is the hrtimer pending? */
+	expires = hrtimer_get_expires(&vcpu->arch.comparecount_timer);
+	if (ktime_compare(now, expires) >= 0) {
+		/*
+		 * Cancel it while we handle it so there's no chance of
+		 * interference with the timeout handler.
+		 */
+		running = hrtimer_cancel(&vcpu->arch.comparecount_timer);
+
+		/* Nothing should be waiting on the timeout */
+		kvm_mips_callbacks->queue_timer_int(vcpu);
+
+		/*
+		 * Restart the timer if it was running based on the expiry time
+		 * we read, so that we don't push it back 2 periods.
+		 */
+		if (running) {
+			expires = ktime_add_ns(expires,
+					       vcpu->arch.count_period);
+			hrtimer_start(&vcpu->arch.comparecount_timer, expires,
+				      HRTIMER_MODE_ABS);
+		}
+	}
+
+	/* Return the biased and scaled guest CP0_Count */
+	return vcpu->arch.count_bias + kvm_mips_ktime_to_count(vcpu, now);
+}
+
+/**
+ * kvm_mips_read_count() - Read the current count value.
+ * @vcpu:	Virtual CPU.
+ *
+ * Read the current guest CP0_Count value, taking into account whether the timer
+ * is stopped.
+ *
+ * Returns:	The current guest CP0_Count value.
+ */
+uint32_t kvm_mips_read_count(struct kvm_vcpu *vcpu)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+
+	/* If count disabled just read static copy of count */
+	if (kvm_mips_count_disabled(vcpu))
+		return kvm_read_c0_guest_count(cop0);
+
+	return kvm_mips_read_count_running(vcpu, ktime_get());
+}
+
+/**
+ * kvm_mips_freeze_hrtimer() - Safely stop the hrtimer.
+ * @vcpu:	Virtual CPU.
+ * @count:	Output pointer for CP0_Count value at point of freeze.
+ *
+ * Freeze the hrtimer safely and return both the ktime and the CP0_Count value
+ * at the point it was frozen. It is guaranteed that any pending interrupts at
+ * the point it was frozen are handled, and none after that point.
+ *
+ * This is useful where the time/CP0_Count is needed in the calculation of the
+ * new parameters.
+ *
+ * Assumes !kvm_mips_count_disabled(@vcpu) (guest CP0_Count timer is running).
+ *
+ * Returns:	The ktime at the point of freeze.
+ */
+static ktime_t kvm_mips_freeze_hrtimer(struct kvm_vcpu *vcpu,
+				       uint32_t *count)
+{
+	ktime_t now;
+
+	/* stop hrtimer before finding time */
+	hrtimer_cancel(&vcpu->arch.comparecount_timer);
+	now = ktime_get();
+
+	/* find count at this point and handle pending hrtimer */
+	*count = kvm_mips_read_count_running(vcpu, now);
+
+	return now;
+}
+
+
+/**
+ * kvm_mips_resume_hrtimer() - Resume hrtimer, updating expiry.
+ * @vcpu:	Virtual CPU.
+ * @now:	ktime at point of resume.
+ * @count:	CP0_Count at point of resume.
+ *
+ * Resumes the timer and updates the timer expiry based on @now and @count.
+ * This can be used in conjunction with kvm_mips_freeze_timer() when timer
+ * parameters need to be changed.
+ *
+ * It is guaranteed that a timer interrupt immediately after resume will be
+ * handled, but not if CP_Compare is exactly at @count. That case is already
+ * handled by kvm_mips_freeze_timer().
+ *
+ * Assumes !kvm_mips_count_disabled(@vcpu) (guest CP0_Count timer is running).
+ */
+static void kvm_mips_resume_hrtimer(struct kvm_vcpu *vcpu,
+				    ktime_t now, uint32_t count)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	uint32_t compare;
+	u64 delta;
+	ktime_t expire;
+
+	/* Calculate timeout (wrap 0 to 2^32) */
+	compare = kvm_read_c0_guest_compare(cop0);
+	delta = (u64)(uint32_t)(compare - count - 1) + 1;
+	delta = div_u64(delta * NSEC_PER_SEC, vcpu->arch.count_hz);
+	expire = ktime_add_ns(now, delta);
+
+	/* Update hrtimer to use new timeout */
+	hrtimer_cancel(&vcpu->arch.comparecount_timer);
+	hrtimer_start(&vcpu->arch.comparecount_timer, expire, HRTIMER_MODE_ABS);
+}
+
+/**
+ * kvm_mips_update_hrtimer() - Update next expiry time of hrtimer.
+ * @vcpu:	Virtual CPU.
+ *
+ * Recalculates and updates the expiry time of the hrtimer. This can be used
+ * after timer parameters have been altered which do not depend on the time that
+ * the change occurs (in those cases kvm_mips_freeze_hrtimer() and
+ * kvm_mips_resume_hrtimer() are used directly).
+ *
+ * It is guaranteed that no timer interrupts will be lost in the process.
+ *
+ * Assumes !kvm_mips_count_disabled(@vcpu) (guest CP0_Count timer is running).
+ */
+static void kvm_mips_update_hrtimer(struct kvm_vcpu *vcpu)
+{
+	ktime_t now;
+	uint32_t count;
+
+	/*
+	 * freeze_hrtimer takes care of a timer interrupts <= count, and
+	 * resume_hrtimer the hrtimer takes care of a timer interrupts > count.
+	 */
+	now = kvm_mips_freeze_hrtimer(vcpu, &count);
+	kvm_mips_resume_hrtimer(vcpu, now, count);
+}
+
+/**
+ * kvm_mips_write_count() - Modify the count and update timer.
+ * @vcpu:	Virtual CPU.
+ * @count:	Guest CP0_Count value to set.
+ *
+ * Sets the CP0_Count value and updates the timer accordingly.
+ */
+void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	ktime_t now;
+	u32 bias;
+
+	/* Calculate bias */
+	now = ktime_get();
+	bias = count - kvm_mips_ktime_to_count(vcpu, now);
+	vcpu->arch.count_bias = bias;
+
+	if (kvm_mips_count_disabled(vcpu))
+		/* The timer's disabled, adjust the static count */
+		kvm_write_c0_guest_count(cop0, count);
+	else
+		/* Update timeout */
+		kvm_mips_resume_hrtimer(vcpu, now, count);
+}
+
+/**
+ * kvm_mips_init_count() - Initialise timer.
+ * @vcpu:	Virtual CPU.
+ *
+ * Initialise the timer to a sensible frequency, namely 100MHz, zero it, and set
+ * it going if it's enabled.
+ */
+void kvm_mips_init_count(struct kvm_vcpu *vcpu)
+{
+	/* 100 MHz */
+	vcpu->arch.count_hz = 100*1000*1000;
+	vcpu->arch.count_period = div_u64((u64)NSEC_PER_SEC << 32,
+					  vcpu->arch.count_hz);
+	vcpu->arch.count_dyn_bias = 0;
+
+	/* Starting at 0 */
+	kvm_mips_write_count(vcpu, 0);
+}
+
+/**
+ * kvm_mips_write_compare() - Modify compare and update timer.
+ * @vcpu:	Virtual CPU.
+ * @compare:	New CP0_Compare value.
+ *
+ * Update CP0_Compare to a new value and update the timeout.
+ */
+void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+
+	/* if unchanged, must just be an ack */
+	if (kvm_read_c0_guest_compare(cop0) == compare)
+		return;
+
+	/* Update compare */
+	kvm_write_c0_guest_compare(cop0, compare);
+
+	/* Update timeout if count enabled */
+	if (!kvm_mips_count_disabled(vcpu))
+		kvm_mips_update_hrtimer(vcpu);
 }
 
 /**
@@ -266,6 +520,94 @@ void kvm_mips_migrate_count(struct kvm_vcpu *vcpu)
 		hrtimer_restart(&vcpu->arch.comparecount_timer);
 }
 
+/**
+ * kvm_mips_count_disable() - Disable count.
+ * @vcpu:	Virtual CPU.
+ *
+ * Disable the CP0_Count timer. A timer interrupt on or before the final stop
+ * time will be handled but not after.
+ *
+ * Assumes CP0_Count was previously enabled but now Guest.CP0_Cause.DC has been
+ * set (count disabled).
+ *
+ * Returns:	The time that the timer was stopped.
+ */
+static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	uint32_t count;
+	ktime_t now;
+
+	/* Stop hrtimer */
+	hrtimer_cancel(&vcpu->arch.comparecount_timer);
+
+	/* Set the static count from the dynamic count, handling pending TI */
+	now = ktime_get();
+	count = kvm_mips_read_count_running(vcpu, now);
+	kvm_write_c0_guest_count(cop0, count);
+
+	return now;
+}
+
+/**
+ * kvm_mips_count_disable_cause() - Disable count using CP0_Cause.DC.
+ * @vcpu:	Virtual CPU.
+ *
+ * Disable the CP0_Count timer and set CP0_Cause.DC. A timer interrupt on or
+ * before the final stop time will be handled, but not after.
+ *
+ * Assumes CP0_Cause.DC is clear (count enabled).
+ */
+void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+
+	kvm_set_c0_guest_cause(cop0, CAUSEF_DC);
+	kvm_mips_count_disable(vcpu);
+}
+
+/**
+ * kvm_mips_count_enable_cause() - Enable count using CP0_Cause.DC.
+ * @vcpu:	Virtual CPU.
+ *
+ * Enable the CP0_Count timer and clear CP0_Cause.DC. A timer interrupt after
+ * the start time will be handled, potentially before even returning, so the
+ * caller should be careful with ordering of CP0_Cause modifications so as not
+ * to lose it.
+ *
+ * Assumes CP0_Cause.DC is set (count disabled).
+ */
+void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	uint32_t count;
+
+	kvm_clear_c0_guest_cause(cop0, CAUSEF_DC);
+
+	/*
+	 * Set the dynamic count to match the static count.
+	 * This starts the hrtimer.
+	 */
+	count = kvm_read_c0_guest_count(cop0);
+	kvm_mips_write_count(vcpu, count);
+}
+
+/**
+ * kvm_mips_count_timeout() - Push timer forward on timeout.
+ * @vcpu:	Virtual CPU.
+ *
+ * Handle an hrtimer event by push the hrtimer forward a period.
+ *
+ * Returns:	The hrtimer_restart value to return to the hrtimer subsystem.
+ */
+enum hrtimer_restart kvm_mips_count_timeout(struct kvm_vcpu *vcpu)
+{
+	/* Add the Count period to the current expiry time */
+	hrtimer_add_expires_ns(&vcpu->arch.comparecount_timer,
+			       vcpu->arch.count_period);
+	return HRTIMER_RESTART;
+}
+
 enum emulation_result kvm_mips_emul_eret(struct kvm_vcpu *vcpu)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
@@ -488,8 +830,7 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
 #endif
 			/* Get reg */
 			if ((rd == MIPS_CP0_COUNT) && (sel == 0)) {
-				/* XXXKYMA: Run the Guest count register @ 1/4 the rate of the host */
-				vcpu->arch.gprs[rt] = (read_c0_count() >> 2);
+				vcpu->arch.gprs[rt] = kvm_mips_read_count(vcpu);
 			} else if ((rd == MIPS_CP0_ERRCTL) && (sel == 0)) {
 				vcpu->arch.gprs[rt] = 0x0;
 #ifdef CONFIG_KVM_MIPS_DYN_TRANS
@@ -556,10 +897,7 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
 			}
 			/* Are we writing to COUNT */
 			else if ((rd == MIPS_CP0_COUNT) && (sel == 0)) {
-				/* Linux doesn't seem to write into COUNT, we throw an error
-				 * if we notice a write to COUNT
-				 */
-				/*er = EMULATE_FAIL; */
+				kvm_mips_write_count(vcpu, vcpu->arch.gprs[rt]);
 				goto done;
 			} else if ((rd == MIPS_CP0_COMPARE) && (sel == 0)) {
 				kvm_debug("[%#x] MTCz, COMPARE %#lx <- %#lx\n",
@@ -569,8 +907,8 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
 				/* If we are writing to COMPARE */
 				/* Clear pending timer interrupt, if any */
 				kvm_mips_callbacks->dequeue_timer_int(vcpu);
-				kvm_write_c0_guest_compare(cop0,
-							   vcpu->arch.gprs[rt]);
+				kvm_mips_write_compare(vcpu,
+						       vcpu->arch.gprs[rt]);
 			} else if ((rd == MIPS_CP0_STATUS) && (sel == 0)) {
 				kvm_write_c0_guest_status(cop0,
 							  vcpu->arch.gprs[rt]);
@@ -581,6 +919,20 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
 #ifdef CONFIG_KVM_MIPS_DYN_TRANS
 				kvm_mips_trans_mtc0(inst, opc, vcpu);
 #endif
+			} else if ((rd == MIPS_CP0_CAUSE) && (sel == 0)) {
+				uint32_t old_cause, new_cause;
+				old_cause = kvm_read_c0_guest_cause(cop0);
+				new_cause = vcpu->arch.gprs[rt];
+				/* Update R/W bits */
+				kvm_change_c0_guest_cause(cop0, 0x08800300,
+							  new_cause);
+				/* DC bit enabling/disabling timer? */
+				if ((old_cause ^ new_cause) & CAUSEF_DC) {
+					if (new_cause & CAUSEF_DC)
+						kvm_mips_count_disable_cause(vcpu);
+					else
+						kvm_mips_count_enable_cause(vcpu);
+				}
 			} else {
 				cop0->reg[rd][sel] = vcpu->arch.gprs[rt];
 #ifdef CONFIG_KVM_MIPS_DYN_TRANS
@@ -1570,8 +1922,7 @@ kvm_mips_handle_ri(unsigned long cause, uint32_t *opc,
 					     current_cpu_data.icache.linesz);
 			break;
 		case 2:	/* Read count register */
-			printk("RDHWR: Cont register\n");
-			arch->gprs[rt] = kvm_read_c0_guest_count(cop0);
+			arch->gprs[rt] = kvm_mips_read_count(vcpu);
 			break;
 		case 3:	/* Count register resolution */
 			switch (current_cpu_data.cputype) {
diff --git a/arch/mips/kvm/kvm_trap_emul.c b/arch/mips/kvm/kvm_trap_emul.c
index f1e8389f8d33..9908f2b0ff46 100644
--- a/arch/mips/kvm/kvm_trap_emul.c
+++ b/arch/mips/kvm/kvm_trap_emul.c
@@ -407,8 +407,7 @@ static int kvm_trap_emul_get_one_reg(struct kvm_vcpu *vcpu,
 {
 	switch (reg->id) {
 	case KVM_REG_MIPS_CP0_COUNT:
-		/* XXXKYMA: Run the Guest count register @ 1/4 the rate of the host */
-		*v = (read_c0_count() >> 2);
+		*v = kvm_mips_read_count(vcpu);
 		break;
 	default:
 		return -EINVAL;
@@ -424,10 +423,30 @@ static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu,
 
 	switch (reg->id) {
 	case KVM_REG_MIPS_CP0_COUNT:
-		/* Not supported yet */
+		kvm_mips_write_count(vcpu, v);
 		break;
 	case KVM_REG_MIPS_CP0_COMPARE:
-		kvm_write_c0_guest_compare(cop0, v);
+		kvm_mips_write_compare(vcpu, v);
+		break;
+	case KVM_REG_MIPS_CP0_CAUSE:
+		/*
+		 * If the timer is stopped or started (DC bit) it must look
+		 * atomic with changes to the interrupt pending bits (TI, IRQ5).
+		 * A timer interrupt should not happen in between.
+		 */
+		if ((kvm_read_c0_guest_cause(cop0) ^ v) & CAUSEF_DC) {
+			if (v & CAUSEF_DC) {
+				/* disable timer first */
+				kvm_mips_count_disable_cause(vcpu);
+				kvm_change_c0_guest_cause(cop0, ~CAUSEF_DC, v);
+			} else {
+				/* enable timer last */
+				kvm_change_c0_guest_cause(cop0, ~CAUSEF_DC, v);
+				kvm_mips_count_enable_cause(vcpu);
+			}
+		} else {
+			kvm_write_c0_guest_cause(cop0, v);
+		}
 		break;
 	default:
 		return -EINVAL;
-- 
1.8.1.2

Re: [PATCH 11/21] MIPS: KVM: Rewrite count/compare timer emulation

[David Daney]

[...]
> diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
> index f56bb699506e..57c1085fd6ab 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -404,8 +404,15 @@ struct kvm_vcpu_arch {
>
>   	u32 io_gpr;		/* GPR used as IO source/target */
>
> -	/* Used to calibrate the virutal count register for the guest */
> -	int32_t host_cp0_count;
> +	struct hrtimer comparecount_timer;
> +	/* Count bias from the raw time */
> +	uint32_t count_bias;
> +	/* Frequency of timer in Hz */
> +	uint32_t count_hz;

We are currently running with timer frequencies of over 2GHz, so the 
width of this variable is close to the limit.

Your follow-on patch exports this to user-space as part of the KVM ABI.

I would suggest, at least for the user-space ABI, to make this a 64-bit 
wide value.


> +	/* Dynamic nanosecond bias (multiple of count_period) to avoid overflow */
> +	s64 count_dyn_bias;
> +	/* Period of timer tick in ns */
> +	u64 count_period;
>
>   	/* Bitmask of exceptions that are pending */
[...]

Re: [PATCH 11/21] MIPS: KVM: Rewrite count/compare timer emulation

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]

Hi David,

On Friday 25 April 2014 10:00:21 David Daney wrote:
> > +	/* Frequency of timer in Hz */
> > +	uint32_t count_hz;
> 
> We are currently running with timer frequencies of over 2GHz, so the
> width of this variable is close to the limit.

Interesting. Do you have 64-bit Count/Compare registers or just accept that 
you'll wrap after only 2 seconds? (it seems like a fairly short maximum idle 
time, though perhaps not important in the grand scheme of things)

> Your follow-on patch exports this to user-space as part of the KVM ABI.
> 
> I would suggest, at least for the user-space ABI, to make this a 64-bit
> wide value.

Yes, it's 64-bit wide in the userspace API, so it should be future proof. 
Currently it refuses to accept a value higher than 1GHz for numerical reasons 
though. If this same timer code is to be adapted for sleeping VZ guests too 
with those sorts of timer frequencies then it makes sense to ensure it can 
handle higher frequencies (since OTOH VZ doesn't give you control over the 
guest timer frequency, just the offset).

It's not strictly required for T&E though so I don't think it should be a 
blocking factor.

Thanks
James

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 12/21] MIPS: KVM: Override guest kernel timer frequency directly

[James Hogan]

The KVM_HOST_FREQ Kconfig symbol was used by KVM guest kernels to
override the timer frequency calculation to a value based on the host
frequency. Now that the KVM timer emulation is implemented independent
of the host timer frequency and defaults to 100MHz, adjust the working
of CONFIG_KVM_HOST_FREQ to match.

The Kconfig symbol now specifies the guest timer frequency directly, and
has been renamed accordingly to KVM_GUEST_TIMER_FREQ. It now defaults to
100MHz too and the help text is updated to make it clear that a zero
value will allow the normal timer frequency calculation to take place
(based on the emulated RTC).

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/Kconfig                | 12 ++++++------
 arch/mips/mti-malta/malta-time.c | 14 ++------------
 2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 5cd695f905a1..5e0014e864f3 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1756,14 +1756,14 @@ config KVM_GUEST
 	help
 	  Select this option if building a guest kernel for KVM (Trap & Emulate) mode
 
-config KVM_HOST_FREQ
-	int "KVM Host Processor Frequency (MHz)"
+config KVM_GUEST_TIMER_FREQ
+	int "Count/Compare Timer Frequency (MHz)"
 	depends on KVM_GUEST
-	default 500
+	default 100
 	help
-	  Select this option if building a guest kernel for KVM to skip
-	  RTC emulation when determining guest CPU Frequency.  Instead, the guest
-	  processor frequency is automatically derived from the host frequency.
+	  Set this to non-zero if building a guest kernel for KVM to skip RTC
+	  emulation when determining guest CPU Frequency. Instead, the guest's
+	  timer frequency is specified directly.
 
 choice
 	prompt "Kernel page size"
diff --git a/arch/mips/mti-malta/malta-time.c b/arch/mips/mti-malta/malta-time.c
index 319009912142..3778a359f3ad 100644
--- a/arch/mips/mti-malta/malta-time.c
+++ b/arch/mips/mti-malta/malta-time.c
@@ -74,18 +74,8 @@ static void __init estimate_frequencies(void)
 	unsigned int giccount = 0, gicstart = 0;
 #endif
 
-#if defined (CONFIG_KVM_GUEST) && defined (CONFIG_KVM_HOST_FREQ)
-	unsigned int prid = read_c0_prid() & (PRID_COMP_MASK | PRID_IMP_MASK);
-
-	/*
-	 * XXXKYMA: hardwire the CPU frequency to Host Freq/4
-	 */
-	count = (CONFIG_KVM_HOST_FREQ * 1000000) >> 3;
-	if ((prid != (PRID_COMP_MIPS | PRID_IMP_20KC)) &&
-	    (prid != (PRID_COMP_MIPS | PRID_IMP_25KF)))
-		count *= 2;
-
-	mips_hpt_frequency = count;
+#if defined(CONFIG_KVM_GUEST) && CONFIG_KVM_GUEST_TIMER_FREQ
+	mips_hpt_frequency = CONFIG_KVM_GUEST_TIMER_FREQ * 1000000;
 	return;
 #endif
 
-- 
1.8.1.2

[PATCH 13/21] MIPS: KVM: Add master disable count interface

[James Hogan]

Expose two new virtual registers to userland via the
KVM_{GET,SET}_ONE_REG ioctls.

KVM_REG_MIPS_COUNT_CTL is for timer configuration fields and just
contains a master disable count bit. This can be used by userland to
freeze the timer in order to read a consistent state from the timer
count value and timer interrupt pending bit. This cannot be done with
the CP0_Cause.DC bit because the timer interrupt pending bit (TI) is
also in CP0_Cause so it would be impossible to stop the timer without
also risking a race with an hrtimer interrupt and having to explicitly
check whether an interrupt should have occurred.

When the timer is re-enabled it resumes without losing time, i.e. the
CP0_Count value jumps to what it would have been had the timer not been
disabled, which would also be impossible to do from userland with
CP0_Cause.DC. The timer interrupt also cannot be lost, i.e. if a timer
interrupt would have occurred had the timer not been disabled it is
queued when the timer is re-enabled.

This works by storing the nanosecond monotonic time when the master
disable is set, and using it for various operations instead of the
current monotonic time (e.g. when recalculating the bias when the
CP0_Count is set), until the master disable is cleared again, i.e. the
timer state is read/written as it would have been at that time. This
state is exposed to userland via the read-only KVM_REG_MIPS_COUNT_RESUME
virtual register so that userland can determine the exact time the
master disable took effect.

This should allow userland to atomically save the state of the timer,
and later restore it.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: David Daney <david.daney@cavium.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h |   5 ++
 arch/mips/include/uapi/asm/kvm.h |  24 +++++++++
 arch/mips/kvm/kvm_mips.c         |   9 +++-
 arch/mips/kvm/kvm_mips_emul.c    | 108 ++++++++++++++++++++++++++++++++++-----
 arch/mips/kvm/kvm_trap_emul.c    |  12 ++++-
 5 files changed, 144 insertions(+), 14 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 57c1085fd6ab..8a6b8f6f44bc 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -405,12 +405,16 @@ struct kvm_vcpu_arch {
 	u32 io_gpr;		/* GPR used as IO source/target */
 
 	struct hrtimer comparecount_timer;
+	/* Count timer control KVM register */
+	uint32_t count_ctl;
 	/* Count bias from the raw time */
 	uint32_t count_bias;
 	/* Frequency of timer in Hz */
 	uint32_t count_hz;
 	/* Dynamic nanosecond bias (multiple of count_period) to avoid overflow */
 	s64 count_dyn_bias;
+	/* Resume time */
+	ktime_t count_resume;
 	/* Period of timer tick in ns */
 	u64 count_period;
 
@@ -713,6 +717,7 @@ uint32_t kvm_mips_read_count(struct kvm_vcpu *vcpu);
 void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count);
 void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare);
 void kvm_mips_init_count(struct kvm_vcpu *vcpu);
+int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl);
 void kvm_mips_migrate_count(struct kvm_vcpu *vcpu);
 void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu);
 void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu);
diff --git a/arch/mips/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
index f09ff5ae2059..1ddc7d7d412a 100644
--- a/arch/mips/include/uapi/asm/kvm.h
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -106,6 +106,30 @@ struct kvm_fpu {
 #define KVM_REG_MIPS_LO (KVM_REG_MIPS | KVM_REG_SIZE_U64 | 33)
 #define KVM_REG_MIPS_PC (KVM_REG_MIPS | KVM_REG_SIZE_U64 | 34)
 
+/* KVM specific control registers */
+
+/*
+ * CP0_Count control
+ * DC:    Set 0: Master disable CP0_Count and set COUNT_RESUME to now
+ *        Set 1: Master re-enable CP0_Count with unchanged bias, handling timer
+ *               interrupts since COUNT_RESUME
+ *        This can be used to freeze the timer to get a consistent snapshot of
+ *        the CP0_Count and timer interrupt pending state, while also resuming
+ *        safely without losing time or guest timer interrupts.
+ * Other: Reserved, do not change.
+ */
+#define KVM_REG_MIPS_COUNT_CTL		(KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
+					 0x20000 | 0)
+#define KVM_REG_MIPS_COUNT_CTL_DC	0x00000001
+/*
+ * CP0_Count resume monotonic nanoseconds
+ * The monotonic nanosecond time of the last set of COUNT_CTL.DC (master
+ * disable). When COUNT_CTL.DC is cleared again (master enable) any timer
+ * interrupts since this time will be emulated.
+ */
+#define KVM_REG_MIPS_COUNT_RESUME	(KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
+					 0x20000 | 1)
+
 /*
  * KVM MIPS specific structures and definitions
  *
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index b32e05efe8b7..f99f8c323b9e 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -545,7 +545,10 @@ static u64 kvm_mips_get_one_regs[] = {
 	KVM_REG_MIPS_CP0_CONFIG2,
 	KVM_REG_MIPS_CP0_CONFIG3,
 	KVM_REG_MIPS_CP0_CONFIG7,
-	KVM_REG_MIPS_CP0_ERROREPC
+	KVM_REG_MIPS_CP0_ERROREPC,
+
+	KVM_REG_MIPS_COUNT_CTL,
+	KVM_REG_MIPS_COUNT_RESUME,
 };
 
 static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
@@ -619,6 +622,8 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 		break;
 	/* registers to be handled specially */
 	case KVM_REG_MIPS_CP0_COUNT:
+	case KVM_REG_MIPS_COUNT_CTL:
+	case KVM_REG_MIPS_COUNT_RESUME:
 		ret = kvm_mips_callbacks->get_one_reg(vcpu, reg, &v);
 		if (ret)
 			return ret;
@@ -708,6 +713,8 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_COUNT:
 	case KVM_REG_MIPS_CP0_COMPARE:
 	case KVM_REG_MIPS_CP0_CAUSE:
+	case KVM_REG_MIPS_COUNT_CTL:
+	case KVM_REG_MIPS_COUNT_RESUME:
 		return kvm_mips_callbacks->set_one_reg(vcpu, reg, v);
 	default:
 		return -EINVAL;
diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index 8ebfd675a3a1..d68af25ffa7f 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -233,14 +233,15 @@ enum emulation_result update_pc(struct kvm_vcpu *vcpu, uint32_t cause)
  * kvm_mips_count_disabled() - Find whether the CP0_Count timer is disabled.
  * @vcpu:	Virtual CPU.
  *
- * Returns:	1 if the CP0_Count timer is disabled by the guest CP0_Cause.DC
- *		bit.
+ * Returns:	1 if the CP0_Count timer is disabled by either the guest
+ *		CP0_Cause.DC bit or the count_ctl.DC bit.
  *		0 otherwise (in which case CP0_Count timer is running).
  */
 static inline int kvm_mips_count_disabled(struct kvm_vcpu *vcpu)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
-	return kvm_read_c0_guest_cause(cop0) & CAUSEF_DC;
+	return	(vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC) ||
+		(kvm_read_c0_guest_cause(cop0) & CAUSEF_DC);
 }
 
 /**
@@ -280,6 +281,24 @@ static uint32_t kvm_mips_ktime_to_count(struct kvm_vcpu *vcpu, ktime_t now)
 }
 
 /**
+ * kvm_mips_count_time() - Get effective current time.
+ * @vcpu:	Virtual CPU.
+ *
+ * Get effective monotonic ktime. This is usually a straightforward ktime_get(),
+ * except when the master disable bit is set in count_ctl, in which case it is
+ * count_resume, i.e. the time that the count was disabled.
+ *
+ * Returns:	Effective monotonic ktime for CP0_Count.
+ */
+static inline ktime_t kvm_mips_count_time(struct kvm_vcpu *vcpu)
+{
+	if (unlikely(vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC))
+		return vcpu->arch.count_resume;
+
+	return ktime_get();
+}
+
+/**
  * kvm_mips_read_count_running() - Read the current count value as if running.
  * @vcpu:	Virtual CPU.
  * @now:	Kernel time to read CP0_Count at.
@@ -449,7 +468,7 @@ void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count)
 	u32 bias;
 
 	/* Calculate bias */
-	now = ktime_get();
+	now = kvm_mips_count_time(vcpu);
 	bias = count - kvm_mips_ktime_to_count(vcpu, now);
 	vcpu->arch.count_bias = bias;
 
@@ -527,8 +546,8 @@ void kvm_mips_migrate_count(struct kvm_vcpu *vcpu)
  * Disable the CP0_Count timer. A timer interrupt on or before the final stop
  * time will be handled but not after.
  *
- * Assumes CP0_Count was previously enabled but now Guest.CP0_Cause.DC has been
- * set (count disabled).
+ * Assumes CP0_Count was previously enabled but now Guest.CP0_Cause.DC or
+ * count_ctl.DC has been set (count disabled).
  *
  * Returns:	The time that the timer was stopped.
  */
@@ -554,7 +573,8 @@ static ktime_t kvm_mips_count_disable(struct kvm_vcpu *vcpu)
  * @vcpu:	Virtual CPU.
  *
  * Disable the CP0_Count timer and set CP0_Cause.DC. A timer interrupt on or
- * before the final stop time will be handled, but not after.
+ * before the final stop time will be handled if the timer isn't disabled by
+ * count_ctl.DC, but not after.
  *
  * Assumes CP0_Cause.DC is clear (count enabled).
  */
@@ -563,7 +583,8 @@ void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
 
 	kvm_set_c0_guest_cause(cop0, CAUSEF_DC);
-	kvm_mips_count_disable(vcpu);
+	if (!(vcpu->arch.count_ctl & KVM_REG_MIPS_COUNT_CTL_DC))
+		kvm_mips_count_disable(vcpu);
 }
 
 /**
@@ -571,9 +592,9 @@ void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu)
  * @vcpu:	Virtual CPU.
  *
  * Enable the CP0_Count timer and clear CP0_Cause.DC. A timer interrupt after
- * the start time will be handled, potentially before even returning, so the
- * caller should be careful with ordering of CP0_Cause modifications so as not
- * to lose it.
+ * the start time will be handled if the timer isn't disabled by count_ctl.DC,
+ * potentially before even returning, so the caller should be careful with
+ * ordering of CP0_Cause modifications so as not to lose it.
  *
  * Assumes CP0_Cause.DC is set (count disabled).
  */
@@ -586,13 +607,76 @@ void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu)
 
 	/*
 	 * Set the dynamic count to match the static count.
-	 * This starts the hrtimer.
+	 * This starts the hrtimer if count_ctl.DC allows it.
+	 * Otherwise it conveniently updates the biases.
 	 */
 	count = kvm_read_c0_guest_count(cop0);
 	kvm_mips_write_count(vcpu, count);
 }
 
 /**
+ * kvm_mips_set_count_ctl() - Update the count control KVM register.
+ * @vcpu:	Virtual CPU.
+ * @count_ctl:	Count control register new value.
+ *
+ * Set the count control KVM register. The timer is updated accordingly.
+ *
+ * Returns:	-EINVAL if reserved bits are set.
+ *		0 on success.
+ */
+int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	s64 changed = count_ctl ^ vcpu->arch.count_ctl;
+	s64 delta;
+	ktime_t expire, now;
+	uint32_t count, compare;
+
+	/* Only allow defined bits to be changed */
+	if (changed & ~(s64)(KVM_REG_MIPS_COUNT_CTL_DC))
+		return -EINVAL;
+
+	/* Apply new value */
+	vcpu->arch.count_ctl = count_ctl;
+
+	/* Master CP0_Count disable */
+	if (changed & KVM_REG_MIPS_COUNT_CTL_DC) {
+		/* Is CP0_Cause.DC already disabling CP0_Count? */
+		if (kvm_read_c0_guest_cause(cop0) & CAUSEF_DC) {
+			if (count_ctl & KVM_REG_MIPS_COUNT_CTL_DC)
+				/* Just record the current time */
+				vcpu->arch.count_resume = ktime_get();
+		} else if (count_ctl & KVM_REG_MIPS_COUNT_CTL_DC) {
+			/* disable timer and record current time */
+			vcpu->arch.count_resume = kvm_mips_count_disable(vcpu);
+		} else {
+			/*
+			 * Calculate timeout relative to static count at resume
+			 * time (wrap 0 to 2^32).
+			 */
+			count = kvm_read_c0_guest_count(cop0);
+			compare = kvm_read_c0_guest_compare(cop0);
+			delta = (u64)(uint32_t)(compare - count - 1) + 1;
+			delta = div_u64(delta * NSEC_PER_SEC,
+					vcpu->arch.count_hz);
+			expire = ktime_add_ns(vcpu->arch.count_resume, delta);
+
+			/* Handle pending interrupt */
+			now = ktime_get();
+			if (ktime_compare(now, expire) >= 0)
+				/* Nothing should be waiting on the timeout */
+				kvm_mips_callbacks->queue_timer_int(vcpu);
+
+			/* Resume hrtimer without changing bias */
+			count = kvm_mips_read_count_running(vcpu, now);
+			kvm_mips_resume_hrtimer(vcpu, now, count);
+		}
+	}
+
+	return 0;
+}
+
+/**
  * kvm_mips_count_timeout() - Push timer forward on timeout.
  * @vcpu:	Virtual CPU.
  *
diff --git a/arch/mips/kvm/kvm_trap_emul.c b/arch/mips/kvm/kvm_trap_emul.c
index 9908f2b0ff46..36fb52f55b11 100644
--- a/arch/mips/kvm/kvm_trap_emul.c
+++ b/arch/mips/kvm/kvm_trap_emul.c
@@ -409,6 +409,12 @@ static int kvm_trap_emul_get_one_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_COUNT:
 		*v = kvm_mips_read_count(vcpu);
 		break;
+	case KVM_REG_MIPS_COUNT_CTL:
+		*v = vcpu->arch.count_ctl;
+		break;
+	case KVM_REG_MIPS_COUNT_RESUME:
+		*v = ktime_to_ns(vcpu->arch.count_resume);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -420,6 +426,7 @@ static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu,
 				     s64 v)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	int ret = 0;
 
 	switch (reg->id) {
 	case KVM_REG_MIPS_CP0_COUNT:
@@ -448,10 +455,13 @@ static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu,
 			kvm_write_c0_guest_cause(cop0, v);
 		}
 		break;
+	case KVM_REG_MIPS_COUNT_CTL:
+		ret = kvm_mips_set_count_ctl(vcpu, v);
+		break;
 	default:
 		return -EINVAL;
 	}
-	return 0;
+	return ret;
 }
 
 static struct kvm_mips_callbacks kvm_trap_emul_callbacks = {
-- 
1.8.1.2

[PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[James Hogan]

Expose the KVM guest CP0_Count bias (from the monotonic kernel time) to
userland in nanosecond units via a new KVM_REG_MIPS_COUNT_BIAS register
accessible with the KVM_{GET,SET}_ONE_REG ioctls. This gives userland
control of the bias so that it can exactly match its own monotonic time.

The nanosecond bias is stored separately from the raw bias used
internally (since nanoseconds isn't a convenient or efficient unit for
various timer calculations), and is recalculated each time the raw count
bias is altered. The raw count bias used in CP0_Count determination is
recalculated when the nanosecond bias is altered via the KVM_SET_ONE_REG
ioctl.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: David Daney <david.daney@cavium.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h |  3 +++
 arch/mips/include/uapi/asm/kvm.h |  9 ++++++++
 arch/mips/kvm/kvm_mips.c         |  3 +++
 arch/mips/kvm/kvm_mips_emul.c    | 46 ++++++++++++++++++++++++++++++++++++++++
 arch/mips/kvm/kvm_trap_emul.c    |  6 ++++++
 5 files changed, 67 insertions(+)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 8a6b8f6f44bc..2a7d4e1be25a 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -413,6 +413,8 @@ struct kvm_vcpu_arch {
 	uint32_t count_hz;
 	/* Dynamic nanosecond bias (multiple of count_period) to avoid overflow */
 	s64 count_dyn_bias;
+	/* Userland nanosecond bias */
+	s64 count_user_bias;
 	/* Resume time */
 	ktime_t count_resume;
 	/* Period of timer tick in ns */
@@ -718,6 +720,7 @@ void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count);
 void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare);
 void kvm_mips_init_count(struct kvm_vcpu *vcpu);
 int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl);
+void kvm_mips_set_count_user_bias(struct kvm_vcpu *vcpu, s64 user_bias);
 void kvm_mips_migrate_count(struct kvm_vcpu *vcpu);
 void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu);
 void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu);
diff --git a/arch/mips/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
index 1ddc7d7d412a..c3d9f6697885 100644
--- a/arch/mips/include/uapi/asm/kvm.h
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -129,6 +129,15 @@ struct kvm_fpu {
  */
 #define KVM_REG_MIPS_COUNT_RESUME	(KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
 					 0x20000 | 1)
+/*
+ * CP0_Count bias against monotonic nanoseconds
+ * This bias added to monotonic nanoseconds and scaled by the frequency gives
+ * the CP0_Count value.
+ * Modified automatically by other changes to CP0_Count.
+ * Modification can result in discontinuous changes in CP0_Count.
+ */
+#define KVM_REG_MIPS_COUNT_BIAS		(KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
+					 0x20000 | 2)
 
 /*
  * KVM MIPS specific structures and definitions
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index f99f8c323b9e..9efb9d4c0882 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -549,6 +549,7 @@ static u64 kvm_mips_get_one_regs[] = {
 
 	KVM_REG_MIPS_COUNT_CTL,
 	KVM_REG_MIPS_COUNT_RESUME,
+	KVM_REG_MIPS_COUNT_BIAS,
 };
 
 static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
@@ -624,6 +625,7 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_COUNT:
 	case KVM_REG_MIPS_COUNT_CTL:
 	case KVM_REG_MIPS_COUNT_RESUME:
+	case KVM_REG_MIPS_COUNT_BIAS:
 		ret = kvm_mips_callbacks->get_one_reg(vcpu, reg, &v);
 		if (ret)
 			return ret;
@@ -715,6 +717,7 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_CAUSE:
 	case KVM_REG_MIPS_COUNT_CTL:
 	case KVM_REG_MIPS_COUNT_RESUME:
+	case KVM_REG_MIPS_COUNT_BIAS:
 		return kvm_mips_callbacks->set_one_reg(vcpu, reg, v);
 	default:
 		return -EINVAL;
diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index d68af25ffa7f..8397f6b3a327 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -466,11 +466,20 @@ void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count)
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
 	ktime_t now;
 	u32 bias;
+	s64 user_bias;
 
 	/* Calculate bias */
 	now = kvm_mips_count_time(vcpu);
 	bias = count - kvm_mips_ktime_to_count(vcpu, now);
+
+	/* Calculate matching user-visible nanosecond bias */
+	user_bias = vcpu->arch.count_dyn_bias +
+			div_u64((u64)bias * NSEC_PER_SEC, vcpu->arch.count_hz);
+	if (ktime_to_ns(now) + user_bias >= vcpu->arch.count_period)
+		user_bias -= vcpu->arch.count_period;
+
 	vcpu->arch.count_bias = bias;
+	vcpu->arch.count_user_bias = user_bias;
 
 	if (kvm_mips_count_disabled(vcpu))
 		/* The timer's disabled, adjust the static count */
@@ -500,6 +509,43 @@ void kvm_mips_init_count(struct kvm_vcpu *vcpu)
 }
 
 /**
+ * kvm_mips_set_count_user_bias() - Set the ns user visible timer bias.
+ * @vcpu:	Virtual CPU.
+ * @user_bias:	User bias in nanoseconds.
+ *
+ * Set the user provided nanosecond bias of the CP0_Count timer, updating the
+ * timer accordingly.
+ */
+void kvm_mips_set_count_user_bias(struct kvm_vcpu *vcpu, s64 user_bias)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	ktime_t now;
+	s64 periods;
+	uint32_t count;
+
+	/* Save new user bias unaltered */
+	vcpu->arch.count_user_bias = user_bias;
+
+	/* Adjust ns bias to be within +/- period */
+	periods = div64_s64(user_bias, vcpu->arch.count_period);
+	user_bias -= periods * vcpu->arch.count_period;
+
+	/* Adjust count_bias = user_bias*hz/1e9 */
+	vcpu->arch.count_bias = div_s64(user_bias * vcpu->arch.count_hz,
+					NSEC_PER_SEC);
+
+	if (kvm_mips_count_disabled(vcpu)) {
+		/* The timer's disabled, adjust the static count */
+		now = kvm_mips_count_time(vcpu);
+		count = kvm_mips_read_count_running(vcpu, now);
+		kvm_write_c0_guest_count(cop0, count);
+	} else {
+		/* Update timeout */
+		kvm_mips_update_hrtimer(vcpu);
+	}
+}
+
+/**
  * kvm_mips_write_compare() - Modify compare and update timer.
  * @vcpu:	Virtual CPU.
  * @compare:	New CP0_Compare value.
diff --git a/arch/mips/kvm/kvm_trap_emul.c b/arch/mips/kvm/kvm_trap_emul.c
index 36fb52f55b11..7aedc822ac09 100644
--- a/arch/mips/kvm/kvm_trap_emul.c
+++ b/arch/mips/kvm/kvm_trap_emul.c
@@ -415,6 +415,9 @@ static int kvm_trap_emul_get_one_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_COUNT_RESUME:
 		*v = ktime_to_ns(vcpu->arch.count_resume);
 		break;
+	case KVM_REG_MIPS_COUNT_BIAS:
+		*v = vcpu->arch.count_user_bias;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -458,6 +461,9 @@ static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_COUNT_CTL:
 		ret = kvm_mips_set_count_ctl(vcpu, v);
 		break;
+	case KVM_REG_MIPS_COUNT_BIAS:
+		kvm_mips_set_count_user_bias(vcpu, v);
+		break;
 	default:
 		return -EINVAL;
 	}
-- 
1.8.1.2

Re: [PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[David Daney]

On 04/25/2014 08:19 AM, James Hogan wrote:
> Expose the KVM guest CP0_Count bias (from the monotonic kernel time) to
> userland in nanosecond units via a new KVM_REG_MIPS_COUNT_BIAS register
> accessible with the KVM_{GET,SET}_ONE_REG ioctls. This gives userland
> control of the bias so that it can exactly match its own monotonic time.
>
> The nanosecond bias is stored separately from the raw bias used
> internally (since nanoseconds isn't a convenient or efficient unit for
> various timer calculations), and is recalculated each time the raw count
> bias is altered. The raw count bias used in CP0_Count determination is
> recalculated when the nanosecond bias is altered via the KVM_SET_ONE_REG
> ioctl.
>

Is this really necessary?

The architecture has CP0_COUNT.  How does the concept of this noew 
synthetic bias value interact with the architecture's CP0_COUNT?

It seems like by adding this you new have two ways to access and 
manipulate the same thing.

1) The architecturally specified CP0_COUNT.
2) This new bias thing.

What if we just let userspace directly manipulate the CP0_COUNT, and if 
necessary only maintain a bias as an internal implementation detail?

David Daney.

> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: David Daney <david.daney@cavium.com>
> Cc: Sanjay Lal <sanjayl@kymasys.com>
> ---
>   arch/mips/include/asm/kvm_host.h |  3 +++
>   arch/mips/include/uapi/asm/kvm.h |  9 ++++++++
>   arch/mips/kvm/kvm_mips.c         |  3 +++
>   arch/mips/kvm/kvm_mips_emul.c    | 46 ++++++++++++++++++++++++++++++++++++++++
>   arch/mips/kvm/kvm_trap_emul.c    |  6 ++++++
>   5 files changed, 67 insertions(+)
>

Re: [PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 4440 bytes --]

Hi David,

On Friday 25 April 2014 10:27:17 David Daney wrote:
> On 04/25/2014 08:19 AM, James Hogan wrote:
> > Expose the KVM guest CP0_Count bias (from the monotonic kernel time) to
> > userland in nanosecond units via a new KVM_REG_MIPS_COUNT_BIAS register
> > accessible with the KVM_{GET,SET}_ONE_REG ioctls. This gives userland
> > control of the bias so that it can exactly match its own monotonic time.
> > 
> > The nanosecond bias is stored separately from the raw bias used
> > internally (since nanoseconds isn't a convenient or efficient unit for
> > various timer calculations), and is recalculated each time the raw count
> > bias is altered. The raw count bias used in CP0_Count determination is
> > recalculated when the nanosecond bias is altered via the KVM_SET_ONE_REG
> > ioctl.
> 
> Is this really necessary?

That's a good question...

> 
> The architecture has CP0_COUNT.  How does the concept of this noew
> synthetic bias value interact with the architecture's CP0_COUNT?

It's a single bias state under the hood for a running timer.

Setting the user_bias effectively results in a guest count of:
CP0_Count = (ktime_to_ns(ktime_get()) + user_bias) * count_hz / 1e9

Under the hood it's actually converted user_bias to a 32-bit bias that 
simplifies the calculations since it wraps more conveniently in places:
CP0_Count = bias + ktime_to_ns(ktime_get()) * count_hz / 1e9

Similarly setting the guest CP0_Count to count results in the bias (and 
user_bias) being recalculated such that CP0_Count at that moment = count:
CP0_Count = count
(substitute CP0_Count)
bias + ktime_to_ns(ktime_get()) * count_hz / 1e9 = count
(rearrange)
bias = count - ktime_to_ns(ktime_get()) * count_hz / 1e9

> 
> It seems like by adding this you new have two ways to access and
> manipulate the same thing.
> 
> 1) The architecturally specified CP0_COUNT.
> 2) This new bias thing.

Almost, but not quite. Full control of the timer value without the new bias is 
a bit more complicated than just writing CP0_COUNT...

> 
> What if we just let userspace directly manipulate the CP0_COUNT, and if
> necessary only maintain a bias as an internal implementation detail?

The difference is in the ability for userland to recalculate the CP0_Count at 
any moment for a running timer (e.g. taking advantage of the fact that I 
believe qemu's qemu_get_clock_ns(rt_clock) = ktime_to_ns(ktime_get()).

Setting the Count directly while the timer is running, the ktime_get() part 
cannot be precisely known to userland.

Since I added the COUNT_CTL.DC & COUNT_RESUME registers it can be partly 
controlled, but only because the timer can be frozen/snapshotted. Userland 
could set COUNT_CTL.DC=1, read COUNT_RESUME to get the time when the timer was 
frozen, then set the Count to the desired value at COUNT_RESUME (which would 
take effect as if the write happened at COUNT_RESUME nanoseconds) and set 
COUNT_CTL.DC=0 to unfreeze the timer. It could then calculate/know the bias 
itself, knowing the CP0_Count at a particular time (COUNT_RESUME) with a 
particular frequency (COUNT_HZ).
Arguably it's simpler (and probably faster) to just write the COUNT_BIAS 
register with a newly calculated or altered bias.

These are I believe the strengths of each related interface:
(1) The master DC provides atomic access to both CP0_Count and CP0_Cause (both 
CP0_Cause.DC and CP0_Cause.TI) which isn't provided by other interfaces.
(2) The COUNT_RESUME in combination with the master DC provides atomic access 
to the current monotonic time and CP0_Count (provided implicitly by (4)), but 
also atomic control of the CP0_Cause at the same time (not provided by other 
interfaces).
(3) Access to the plain CP0_Count provides straightforward access to the 
CP0_Count of a running or more importantly stopped timer.
(4) Access to the bias provides exact control of the value of the timer 
relative to monotonic time while the timer is running (provided implicitly by 
(2)), without having to freeze it (not provided elsewhere).

So yes, you could technically manage without (4) by using (2) ((4) was 
implemented first), but I think it probably still has some value since you can 
do it with a single ioctl rather than 4 ioctls (freeze timer, read 
resume_time, read or write count, unfreeze timer).

Enough value to be worthwhile? I haven't really made up my mind yet but I'm 
leaning towards yes.

Do you have any further thoughts on that?

Thanks
James

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[Paolo Bonzini]

Il 26/04/2014 00:34, James Hogan ha scritto:
> So yes, you could technically manage without (4) by using (2) ((4) was
> implemented first), but I think it probably still has some value since you can
> do it with a single ioctl rather than 4 ioctls (freeze timer, read
> resume_time, read or write count, unfreeze timer).
>
> Enough value to be worthwhile? I haven't really made up my mind yet but I'm
> leaning towards yes.

It would be interesting to see how the userspace patches use this 
independent of COUNT_RESUME.

Paolo

Re: [PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[James Hogan]

Hi Paolo, David, Andreas,

On 26/04/14 10:37, Paolo Bonzini wrote:
> Il 26/04/2014 00:34, James Hogan ha scritto:
>> So yes, you could technically manage without (4) by using (2) ((4) was
>> implemented first), but I think it probably still has some value since
>> you can
>> do it with a single ioctl rather than 4 ioctls (freeze timer, read
>> resume_time, read or write count, unfreeze timer).
>>
>> Enough value to be worthwhile? I haven't really made up my mind yet
>> but I'm
>> leaning towards yes.
> 
> It would be interesting to see how the userspace patches use this
> independent of COUNT_RESUME.

The implementation in QEMU that I've settled upon makes do with just
COUNT_CTL and COUNT_RESUME, but with a slight kernel modification so
that COUNT_RESUME is writeable (to any positive monotonic nanosecond
value <= now). It works fairly cleanly and correctly even with stopping
and starting VM clock (gdb, stop/cont, savevm/loadvm, live migration),
to match the behaviour of the existing mips cpu timer emulation, so I
plan to drop this bias patch, and will post a v2 patchset soon with just
a few modifications.

QEMU saves the state of the KVM timer from kvm_arch_get_registers() or
when the VM clock is stopped (via a vmstate notifier) - whichever comes
first. It then restores the KVM timer from kvm_arch_put_registers() or
when the VM clock is started - whichever comes last.
Example sequence:
stop VM - SAVE
get regs - vm clock already stopped, not saved again
start VM - regs dirty, not restored
put regs - vm clock running, RESTORE

Saving involves:
COUNT_CTL.DC = 1 (freeze KVM timer)
get CP0_Cause, CP0_Count and COUNT_RESUME
store a copy of the calculated VM clock @COUNT_RESUME nanoseconds
(i.e. the VM clock corresponding to the saved CP0_Count)

Restoring involves:
put COUNT_RESUME = now - (vm clock @now - saved vm clock)
(resume occurs at the same interval into the past that the VM clock has
increased since saving)
put CP0_Cause, CP0_Count
(the stored CP0_Count applies at that resume time)
COUNT_CTL.DC = 0 (resume KVM timer from CP0_Count at COUNT_RESUME)

I'll post an updated QEMU patchset ASAP after the KVM patchset, but
wanted to explain how this API can actually be used. Does it sound
reasonable?

Thanks
James

Re: [PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[Paolo Bonzini]

Il 28/05/2014 16:21, James Hogan ha scritto:
> The implementation in QEMU that I've settled upon makes do with just
> COUNT_CTL and COUNT_RESUME, but with a slight kernel modification so
> that COUNT_RESUME is writeable (to any positive monotonic nanosecond
> value <= now). It works fairly cleanly and correctly even with stopping
> and starting VM clock (gdb, stop/cont, savevm/loadvm, live migration),
> to match the behaviour of the existing mips cpu timer emulation, so I
> plan to drop this bias patch, and will post a v2 patchset soon with just
> a few modifications.

It makes sense to have writable registers in the emulator, even if they 
are read-only in real hardware.  We also do that for x86, FWIW.

So the idea looks okay to me.

Paolo

Re: [PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[Paolo Bonzini]

Il 25/04/2014 17:19, James Hogan ha scritto:
> Expose the KVM guest CP0_Count bias (from the monotonic kernel time) to
> userland in nanosecond units via a new KVM_REG_MIPS_COUNT_BIAS register
> accessible with the KVM_{GET,SET}_ONE_REG ioctls. This gives userland
> control of the bias so that it can exactly match its own monotonic time.
>
> The nanosecond bias is stored separately from the raw bias used
> internally (since nanoseconds isn't a convenient or efficient unit for
> various timer calculations), and is recalculated each time the raw count
> bias is altered. The raw count bias used in CP0_Count determination is
> recalculated when the nanosecond bias is altered via the KVM_SET_ONE_REG
> ioctl.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: David Daney <david.daney@cavium.com>
> Cc: Sanjay Lal <sanjayl@kymasys.com>

If it is possible and not too hairy to use a raw value in userspace 
(together with KVM_REG_MIPS_COUNT_HZ), please do it---my suggestions 
were just that, a suggestion.  Otherwise, the patch looks good.

Paolo

Re: [PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[James Hogan]

Hi Paolo,

On 28 April 2014 13:01, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 25/04/2014 17:19, James Hogan ha scritto:
>
>> Expose the KVM guest CP0_Count bias (from the monotonic kernel time) to
>> userland in nanosecond units via a new KVM_REG_MIPS_COUNT_BIAS register
>> accessible with the KVM_{GET,SET}_ONE_REG ioctls. This gives userland
>> control of the bias so that it can exactly match its own monotonic time.
>>
>> The nanosecond bias is stored separately from the raw bias used
>> internally (since nanoseconds isn't a convenient or efficient unit for
>> various timer calculations), and is recalculated each time the raw count
>> bias is altered. The raw count bias used in CP0_Count determination is
>> recalculated when the nanosecond bias is altered via the KVM_SET_ONE_REG
>> ioctl.
>>
>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Gleb Natapov <gleb@kernel.org>
>> Cc: kvm@vger.kernel.org
>> Cc: Ralf Baechle <ralf@linux-mips.org>
>> Cc: linux-mips@linux-mips.org
>> Cc: David Daney <david.daney@cavium.com>
>> Cc: Sanjay Lal <sanjayl@kymasys.com>
>
>
> If it is possible and not too hairy to use a raw value in userspace
> (together with KVM_REG_MIPS_COUNT_HZ), please do it---my suggestions were
> just that, a suggestion.  Otherwise, the patch looks good.

Do you mean expose the raw internal offset to userland instead of the
nanosecond one? Yeh it should be possible & slightly simpler for both
kernel and Qemu actually.

Qemu could then store that value (or the Count register) straight into
env->CP0_Count (depending on Cause.DC), then hw/mips/cputimer.c would
pretty much continue to work accurately. cputimer.c is only really
made use of by tcg at the moment though (reading/writing
count/compare/cause.DC), but it still makes sense to be consistent.

Cheers
James

Re: [PATCH 14/21] MIPS: KVM: Add nanosecond count bias KVM register

[Paolo Bonzini]

Il 28/04/2014 17:17, James Hogan ha scritto:
>> > If it is possible and not too hairy to use a raw value in userspace
>> > (together with KVM_REG_MIPS_COUNT_HZ), please do it---my suggestions were
>> > just that, a suggestion.  Otherwise, the patch looks good.
> Do you mean expose the raw internal offset to userland instead of the
> nanosecond one? Yeh it should be possible & slightly simpler for both
> kernel and Qemu actually.

Yes, when I reviewed the QEMU patches I missed this consequence of 
exposing the HZ.

> Qemu could then store that value (or the Count register) straight into
> env->CP0_Count (depending on Cause.DC), then hw/mips/cputimer.c would
> pretty much continue to work accurately. cputimer.c is only really
> made use of by tcg at the moment though (reading/writing
> count/compare/cause.DC), but it still makes sense to be consistent.

Yup.  cputimer.c would just use a HZ value stored in CPUMIPSState 
(TIMER_FREQ for TCG) instead of hardcoding TIMER_FREQ I guess.

Paolo

[PATCH 15/21] MIPS: KVM: Add count frequency KVM register

[James Hogan]

Expose the KVM guest CP0_Count frequency to userland via a new
KVM_REG_MIPS_COUNT_HZ register accessible with the KVM_{GET,SET}_ONE_REG
ioctls.

When the frequency is altered the bias is adjusted such that the guest
CP0_Count doesn't jump discontinuously or lose any timer interrupts.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: David Daney <david.daney@cavium.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h |  1 +
 arch/mips/include/uapi/asm/kvm.h |  7 +++++
 arch/mips/kvm/kvm_mips.c         |  3 +++
 arch/mips/kvm/kvm_mips_emul.c    | 58 ++++++++++++++++++++++++++++++++++++++++
 arch/mips/kvm/kvm_trap_emul.c    |  6 +++++
 5 files changed, 75 insertions(+)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 2a7d4e1be25a..94a7299e1f6c 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -721,6 +721,7 @@ void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare);
 void kvm_mips_init_count(struct kvm_vcpu *vcpu);
 int kvm_mips_set_count_ctl(struct kvm_vcpu *vcpu, s64 count_ctl);
 void kvm_mips_set_count_user_bias(struct kvm_vcpu *vcpu, s64 user_bias);
+int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz);
 void kvm_mips_migrate_count(struct kvm_vcpu *vcpu);
 void kvm_mips_count_enable_cause(struct kvm_vcpu *vcpu);
 void kvm_mips_count_disable_cause(struct kvm_vcpu *vcpu);
diff --git a/arch/mips/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
index c3d9f6697885..f9487f8b3394 100644
--- a/arch/mips/include/uapi/asm/kvm.h
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -138,6 +138,13 @@ struct kvm_fpu {
  */
 #define KVM_REG_MIPS_COUNT_BIAS		(KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
 					 0x20000 | 2)
+/*
+ * CP0_Count rate in Hz
+ * Specifies the rate of the CP0_Count timer in Hz. Modifications occur without
+ * discontinuities in CP0_Count.
+ */
+#define KVM_REG_MIPS_COUNT_HZ		(KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
+					 0x20000 | 3)
 
 /*
  * KVM MIPS specific structures and definitions
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 9efb9d4c0882..cbb3cdca2878 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -550,6 +550,7 @@ static u64 kvm_mips_get_one_regs[] = {
 	KVM_REG_MIPS_COUNT_CTL,
 	KVM_REG_MIPS_COUNT_RESUME,
 	KVM_REG_MIPS_COUNT_BIAS,
+	KVM_REG_MIPS_COUNT_HZ,
 };
 
 static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
@@ -626,6 +627,7 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_COUNT_CTL:
 	case KVM_REG_MIPS_COUNT_RESUME:
 	case KVM_REG_MIPS_COUNT_BIAS:
+	case KVM_REG_MIPS_COUNT_HZ:
 		ret = kvm_mips_callbacks->get_one_reg(vcpu, reg, &v);
 		if (ret)
 			return ret;
@@ -718,6 +720,7 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_COUNT_CTL:
 	case KVM_REG_MIPS_COUNT_RESUME:
 	case KVM_REG_MIPS_COUNT_BIAS:
+	case KVM_REG_MIPS_COUNT_HZ:
 		return kvm_mips_callbacks->set_one_reg(vcpu, reg, v);
 	default:
 		return -EINVAL;
diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index 8397f6b3a327..fcbc5ff6c347 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -546,6 +546,64 @@ void kvm_mips_set_count_user_bias(struct kvm_vcpu *vcpu, s64 user_bias)
 }
 
 /**
+ * kvm_mips_set_count_hz() - Update the frequency of the timer.
+ * @vcpu:	Virtual CPU.
+ * @count_hz:	Frequency of CP0_Count timer in Hz.
+ *
+ * Change the frequency of the CP0_Count timer. This is done atomically so that
+ * CP0_Count is continuous and no timer interrupt is lost.
+ *
+ * Returns:	-EINVAL if @count_hz is out of range.
+ *		0 on success.
+ */
+int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz)
+{
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	int dc;
+	ktime_t now;
+	u32 count, bias;
+	s64 user_bias;
+
+	/* ensure the frequency is in a sensible range... */
+	if (count_hz <= 0 || count_hz > NSEC_PER_SEC)
+		return -EINVAL;
+	/* ... and has actually changed */
+	if (vcpu->arch.count_hz == count_hz)
+		return 0;
+
+	/* Safely freeze timer so we can keep it continuous */
+	dc = kvm_mips_count_disabled(vcpu);
+	if (dc) {
+		now = kvm_mips_count_time(vcpu);
+		count = kvm_read_c0_guest_count(cop0);
+	} else {
+		now = kvm_mips_freeze_hrtimer(vcpu, &count);
+	}
+
+	/* Update the frequency */
+	vcpu->arch.count_hz = count_hz;
+	vcpu->arch.count_period = div_u64((u64)NSEC_PER_SEC << 32, count_hz);
+	vcpu->arch.count_dyn_bias = 0;
+
+	/* Calculate adjusted bias so dynamic count is unchanged */
+	bias = count - kvm_mips_ktime_to_count(vcpu, now);
+
+	/* Calculate matching user-visible nanosecond bias */
+	user_bias = vcpu->arch.count_dyn_bias +
+			div_u64((u64)bias * NSEC_PER_SEC, count_hz);
+	if (ktime_to_ns(now) + user_bias >= vcpu->arch.count_period)
+		user_bias -= vcpu->arch.count_period;
+
+	vcpu->arch.count_bias = bias;
+	vcpu->arch.count_user_bias = user_bias;
+
+	/* Update and resume hrtimer */
+	if (!dc)
+		kvm_mips_resume_hrtimer(vcpu, now, count);
+	return 0;
+}
+
+/**
  * kvm_mips_write_compare() - Modify compare and update timer.
  * @vcpu:	Virtual CPU.
  * @compare:	New CP0_Compare value.
diff --git a/arch/mips/kvm/kvm_trap_emul.c b/arch/mips/kvm/kvm_trap_emul.c
index 7aedc822ac09..30cef8e6da81 100644
--- a/arch/mips/kvm/kvm_trap_emul.c
+++ b/arch/mips/kvm/kvm_trap_emul.c
@@ -418,6 +418,9 @@ static int kvm_trap_emul_get_one_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_COUNT_BIAS:
 		*v = vcpu->arch.count_user_bias;
 		break;
+	case KVM_REG_MIPS_COUNT_HZ:
+		*v = vcpu->arch.count_hz;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -464,6 +467,9 @@ static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_COUNT_BIAS:
 		kvm_mips_set_count_user_bias(vcpu, v);
 		break;
+	case KVM_REG_MIPS_COUNT_HZ:
+		ret = kvm_mips_set_count_hz(vcpu, v);
+		break;
 	default:
 		return -EINVAL;
 	}
-- 
1.8.1.2

[PATCH 16/21] MIPS: KVM: Make kvm_mips_comparecount_{func,wakeup} static

[James Hogan]

The kvm_mips_comparecount_func() and kvm_mips_comparecount_wakeup()
functions are only used within arch/mips/kvm/kvm_mips.c, so make them
static.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_mips.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index cbb3cdca2878..0cf03efe8a57 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -973,7 +973,7 @@ int kvm_arch_vcpu_ioctl_get_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
 	return 0;
 }
 
-void kvm_mips_comparecount_func(unsigned long data)
+static void kvm_mips_comparecount_func(unsigned long data)
 {
 	struct kvm_vcpu *vcpu = (struct kvm_vcpu *)data;
 
@@ -988,7 +988,7 @@ void kvm_mips_comparecount_func(unsigned long data)
 /*
  * low level hrtimer wake routine.
  */
-enum hrtimer_restart kvm_mips_comparecount_wakeup(struct hrtimer *timer)
+static enum hrtimer_restart kvm_mips_comparecount_wakeup(struct hrtimer *timer)
 {
 	struct kvm_vcpu *vcpu;
 
-- 
1.8.1.2

[PATCH 17/21] MIPS: KVM: Whitespace fixes in kvm_mips_callbacks

[James Hogan]

Fix whitespace in struct kvm_mips_callbacks function pointers.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/include/asm/kvm_host.h | 46 ++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 94a7299e1f6c..3d6263d4d9fa 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -570,29 +570,29 @@ static inline void _kvm_atomic_change_c0_guest_reg(unsigned long *reg,
 
 
 struct kvm_mips_callbacks {
-	int (*handle_cop_unusable) (struct kvm_vcpu *vcpu);
-	int (*handle_tlb_mod) (struct kvm_vcpu *vcpu);
-	int (*handle_tlb_ld_miss) (struct kvm_vcpu *vcpu);
-	int (*handle_tlb_st_miss) (struct kvm_vcpu *vcpu);
-	int (*handle_addr_err_st) (struct kvm_vcpu *vcpu);
-	int (*handle_addr_err_ld) (struct kvm_vcpu *vcpu);
-	int (*handle_syscall) (struct kvm_vcpu *vcpu);
-	int (*handle_res_inst) (struct kvm_vcpu *vcpu);
-	int (*handle_break) (struct kvm_vcpu *vcpu);
-	int (*vm_init) (struct kvm *kvm);
-	int (*vcpu_init) (struct kvm_vcpu *vcpu);
-	int (*vcpu_setup) (struct kvm_vcpu *vcpu);
-	 gpa_t(*gva_to_gpa) (gva_t gva);
-	void (*queue_timer_int) (struct kvm_vcpu *vcpu);
-	void (*dequeue_timer_int) (struct kvm_vcpu *vcpu);
-	void (*queue_io_int) (struct kvm_vcpu *vcpu,
-			      struct kvm_mips_interrupt *irq);
-	void (*dequeue_io_int) (struct kvm_vcpu *vcpu,
-				struct kvm_mips_interrupt *irq);
-	int (*irq_deliver) (struct kvm_vcpu *vcpu, unsigned int priority,
-			    uint32_t cause);
-	int (*irq_clear) (struct kvm_vcpu *vcpu, unsigned int priority,
-			  uint32_t cause);
+	int (*handle_cop_unusable)(struct kvm_vcpu *vcpu);
+	int (*handle_tlb_mod)(struct kvm_vcpu *vcpu);
+	int (*handle_tlb_ld_miss)(struct kvm_vcpu *vcpu);
+	int (*handle_tlb_st_miss)(struct kvm_vcpu *vcpu);
+	int (*handle_addr_err_st)(struct kvm_vcpu *vcpu);
+	int (*handle_addr_err_ld)(struct kvm_vcpu *vcpu);
+	int (*handle_syscall)(struct kvm_vcpu *vcpu);
+	int (*handle_res_inst)(struct kvm_vcpu *vcpu);
+	int (*handle_break)(struct kvm_vcpu *vcpu);
+	int (*vm_init)(struct kvm *kvm);
+	int (*vcpu_init)(struct kvm_vcpu *vcpu);
+	int (*vcpu_setup)(struct kvm_vcpu *vcpu);
+	gpa_t (*gva_to_gpa)(gva_t gva);
+	void (*queue_timer_int)(struct kvm_vcpu *vcpu);
+	void (*dequeue_timer_int)(struct kvm_vcpu *vcpu);
+	void (*queue_io_int)(struct kvm_vcpu *vcpu,
+			     struct kvm_mips_interrupt *irq);
+	void (*dequeue_io_int)(struct kvm_vcpu *vcpu,
+			       struct kvm_mips_interrupt *irq);
+	int (*irq_deliver)(struct kvm_vcpu *vcpu, unsigned int priority,
+			   uint32_t cause);
+	int (*irq_clear)(struct kvm_vcpu *vcpu, unsigned int priority,
+			 uint32_t cause);
 	int (*get_one_reg)(struct kvm_vcpu *vcpu,
 			   const struct kvm_one_reg *reg, s64 *v);
 	int (*set_one_reg)(struct kvm_vcpu *vcpu,
-- 
1.8.1.2

[PATCH 18/21] MIPS: KVM: Fix kvm_debug bit-rottage

[James Hogan]

Fix build errors when DEBUG is defined in arch/mips/kvm/.
 - The DEBUG code in kvm_mips_handle_tlbmod() was missing some variables.
 - The DEBUG code in kvm_mips_host_tlb_write() was conditional on an
   undefined "debug" variable.
 - The DEBUG code in kvm_mips_host_tlb_inv() accessed asid_map directly
   rather than using kvm_mips_get_user_asid(). Also fixed brace
   placement.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_mips_emul.c |  6 +++++-
 arch/mips/kvm/kvm_tlb.c       | 14 +++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index fcbc5ff6c347..ba7519b78985 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -1882,8 +1882,12 @@ kvm_mips_handle_tlbmod(unsigned long cause, uint32_t *opc,
 		       struct kvm_run *run, struct kvm_vcpu *vcpu)
 {
 	enum emulation_result er = EMULATE_DONE;
-
 #ifdef DEBUG
+	struct mips_coproc *cop0 = vcpu->arch.cop0;
+	unsigned long entryhi = (vcpu->arch.host_cp0_badvaddr & VPN2_MASK) |
+				(kvm_read_c0_guest_entryhi(cop0) & ASID_MASK);
+	int index;
+
 	/*
 	 * If address not in the guest TLB, then we are in trouble
 	 */
diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 1f61fe6739da..53a39d32b5e0 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -233,12 +233,9 @@ kvm_mips_host_tlb_write(struct kvm_vcpu *vcpu, unsigned long entryhi,
 	tlbw_use_hazard();
 
 #ifdef DEBUG
-	if (debug) {
-		kvm_debug("@ %#lx idx: %2d [entryhi(R): %#lx] "
-			  "entrylo0(R): 0x%08lx, entrylo1(R): 0x%08lx\n",
-			  vcpu->arch.pc, idx, read_c0_entryhi(),
-			  read_c0_entrylo0(), read_c0_entrylo1());
-	}
+	kvm_debug("@ %#lx idx: %2d [entryhi(R): %#lx] entrylo0(R): 0x%08lx, entrylo1(R): 0x%08lx\n",
+		  vcpu->arch.pc, idx, read_c0_entryhi(),
+		  read_c0_entrylo0(), read_c0_entrylo1());
 #endif
 
 	/* Flush D-cache */
@@ -507,10 +504,9 @@ int kvm_mips_host_tlb_inv(struct kvm_vcpu *vcpu, unsigned long va)
 	local_irq_restore(flags);
 
 #ifdef DEBUG
-	if (idx > 0) {
+	if (idx > 0)
 		kvm_debug("%s: Invalidated entryhi %#lx @ idx %d\n", __func__,
-			  (va & VPN2_MASK) | (vcpu->arch.asid_map[va & ASID_MASK] & ASID_MASK), idx);
-	}
+			  (va & VPN2_MASK) | kvm_mips_get_user_asid(vcpu), idx);
 #endif
 
 	return 0;
-- 
1.8.1.2

[PATCH 19/21] MIPS: KVM: Remove ifdef DEBUG around kvm_debug

[James Hogan]

kvm_debug() uses pr_debug() which is already compiled out in the absence
of a DEBUG define, so remove the unnecessary ifdef DEBUG lines around
kvm_debug() calls which are littered around arch/mips/kvm/.

As well as generally cleaning up, this prevents future bit-rot due to
DEBUG not being commonly used.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_mips_emul.c |  2 --
 arch/mips/kvm/kvm_tlb.c       | 14 --------------
 arch/mips/kvm/kvm_trap_emul.c | 12 ------------
 3 files changed, 28 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index ba7519b78985..698f4cb47d7e 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -2370,11 +2370,9 @@ kvm_mips_handle_tlbmiss(unsigned long cause, uint32_t *opc,
 				er = EMULATE_FAIL;
 			}
 		} else {
-#ifdef DEBUG
 			kvm_debug
 			    ("Injecting hi: %#lx, lo0: %#lx, lo1: %#lx into shadow host TLB\n",
 			     tlb->tlb_hi, tlb->tlb_lo0, tlb->tlb_lo1);
-#endif
 			/* OK we have a Guest TLB entry, now inject it into the shadow host TLB */
 			kvm_mips_handle_mapped_seg_tlb_fault(vcpu, tlb, NULL,
 							     NULL);
diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 53a39d32b5e0..51e7866ea893 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -232,11 +232,9 @@ kvm_mips_host_tlb_write(struct kvm_vcpu *vcpu, unsigned long entryhi,
 		tlb_write_indexed();
 	tlbw_use_hazard();
 
-#ifdef DEBUG
 	kvm_debug("@ %#lx idx: %2d [entryhi(R): %#lx] entrylo0(R): 0x%08lx, entrylo1(R): 0x%08lx\n",
 		  vcpu->arch.pc, idx, read_c0_entryhi(),
 		  read_c0_entrylo0(), read_c0_entrylo1());
-#endif
 
 	/* Flush D-cache */
 	if (flush_dcache_mask) {
@@ -343,11 +341,9 @@ int kvm_mips_handle_commpage_tlb_fault(unsigned long badvaddr,
 	mtc0_tlbw_hazard();
 	tlbw_use_hazard();
 
-#ifdef DEBUG
 	kvm_debug ("@ %#lx idx: %2d [entryhi(R): %#lx] entrylo0 (R): 0x%08lx, entrylo1(R): 0x%08lx\n",
 	     vcpu->arch.pc, read_c0_index(), read_c0_entryhi(),
 	     read_c0_entrylo0(), read_c0_entrylo1());
-#endif
 
 	/* Restore old ASID */
 	write_c0_entryhi(old_entryhi);
@@ -395,10 +391,8 @@ kvm_mips_handle_mapped_seg_tlb_fault(struct kvm_vcpu *vcpu,
 	entrylo1 = mips3_paddr_to_tlbpfn(pfn1 << PAGE_SHIFT) | (0x3 << 3) |
 			(tlb->tlb_lo1 & MIPS3_PG_D) | (tlb->tlb_lo1 & MIPS3_PG_V);
 
-#ifdef DEBUG
 	kvm_debug("@ %#lx tlb_lo0: 0x%08lx tlb_lo1: 0x%08lx\n", vcpu->arch.pc,
 		  tlb->tlb_lo0, tlb->tlb_lo1);
-#endif
 
 	return kvm_mips_host_tlb_write(vcpu, entryhi, entrylo0, entrylo1,
 				       tlb->tlb_mask);
@@ -419,10 +413,8 @@ int kvm_mips_guest_tlb_lookup(struct kvm_vcpu *vcpu, unsigned long entryhi)
 		}
 	}
 
-#ifdef DEBUG
 	kvm_debug("%s: entryhi: %#lx, index: %d lo0: %#lx, lo1: %#lx\n",
 		  __func__, entryhi, index, tlb[i].tlb_lo0, tlb[i].tlb_lo1);
-#endif
 
 	return index;
 }
@@ -456,9 +448,7 @@ int kvm_mips_host_tlb_lookup(struct kvm_vcpu *vcpu, unsigned long vaddr)
 
 	local_irq_restore(flags);
 
-#ifdef DEBUG
 	kvm_debug("Host TLB lookup, %#lx, idx: %2d\n", vaddr, idx);
-#endif
 
 	return idx;
 }
@@ -503,11 +493,9 @@ int kvm_mips_host_tlb_inv(struct kvm_vcpu *vcpu, unsigned long va)
 
 	local_irq_restore(flags);
 
-#ifdef DEBUG
 	if (idx > 0)
 		kvm_debug("%s: Invalidated entryhi %#lx @ idx %d\n", __func__,
 			  (va & VPN2_MASK) | kvm_mips_get_user_asid(vcpu), idx);
-#endif
 
 	return 0;
 }
@@ -658,9 +646,7 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	unsigned long flags;
 	int newasid = 0;
 
-#ifdef DEBUG
 	kvm_debug("%s: vcpu %p, cpu: %d\n", __func__, vcpu, cpu);
-#endif
 
 	/* Alocate new kernel and user ASIDs if needed */
 
diff --git a/arch/mips/kvm/kvm_trap_emul.c b/arch/mips/kvm/kvm_trap_emul.c
index 30cef8e6da81..04ed46783a45 100644
--- a/arch/mips/kvm/kvm_trap_emul.c
+++ b/arch/mips/kvm/kvm_trap_emul.c
@@ -32,9 +32,7 @@ static gpa_t kvm_trap_emul_gva_to_gpa_cb(gva_t gva)
 		gpa = KVM_INVALID_ADDR;
 	}
 
-#ifdef DEBUG
 	kvm_debug("%s: gva %#lx, gpa: %#llx\n", __func__, gva, gpa);
-#endif
 
 	return gpa;
 }
@@ -85,11 +83,9 @@ static int kvm_trap_emul_handle_tlb_mod(struct kvm_vcpu *vcpu)
 
 	if (KVM_GUEST_KSEGX(badvaddr) < KVM_GUEST_KSEG0
 	    || KVM_GUEST_KSEGX(badvaddr) == KVM_GUEST_KSEG23) {
-#ifdef DEBUG
 		kvm_debug
 		    ("USER/KSEG23 ADDR TLB MOD fault: cause %#lx, PC: %p, BadVaddr: %#lx\n",
 		     cause, opc, badvaddr);
-#endif
 		er = kvm_mips_handle_tlbmod(cause, opc, run, vcpu);
 
 		if (er == EMULATE_DONE)
@@ -138,11 +134,9 @@ static int kvm_trap_emul_handle_tlb_st_miss(struct kvm_vcpu *vcpu)
 		}
 	} else if (KVM_GUEST_KSEGX(badvaddr) < KVM_GUEST_KSEG0
 		   || KVM_GUEST_KSEGX(badvaddr) == KVM_GUEST_KSEG23) {
-#ifdef DEBUG
 		kvm_debug
 		    ("USER ADDR TLB LD fault: cause %#lx, PC: %p, BadVaddr: %#lx\n",
 		     cause, opc, badvaddr);
-#endif
 		er = kvm_mips_handle_tlbmiss(cause, opc, run, vcpu);
 		if (er == EMULATE_DONE)
 			ret = RESUME_GUEST;
@@ -188,10 +182,8 @@ static int kvm_trap_emul_handle_tlb_ld_miss(struct kvm_vcpu *vcpu)
 		}
 	} else if (KVM_GUEST_KSEGX(badvaddr) < KVM_GUEST_KSEG0
 		   || KVM_GUEST_KSEGX(badvaddr) == KVM_GUEST_KSEG23) {
-#ifdef DEBUG
 		kvm_debug("USER ADDR TLB ST fault: PC: %#lx, BadVaddr: %#lx\n",
 			  vcpu->arch.pc, badvaddr);
-#endif
 
 		/* User Address (UA) fault, this could happen if
 		 * (1) TLB entry not present/valid in both Guest and shadow host TLBs, in this
@@ -236,9 +228,7 @@ static int kvm_trap_emul_handle_addr_err_st(struct kvm_vcpu *vcpu)
 
 	if (KVM_GUEST_KERNEL_MODE(vcpu)
 	    && (KSEGX(badvaddr) == CKSEG0 || KSEGX(badvaddr) == CKSEG1)) {
-#ifdef DEBUG
 		kvm_debug("Emulate Store to MMIO space\n");
-#endif
 		er = kvm_mips_emulate_inst(cause, opc, run, vcpu);
 		if (er == EMULATE_FAIL) {
 			printk("Emulate Store to MMIO space failed\n");
@@ -268,9 +258,7 @@ static int kvm_trap_emul_handle_addr_err_ld(struct kvm_vcpu *vcpu)
 	int ret = RESUME_GUEST;
 
 	if (KSEGX(badvaddr) == CKSEG0 || KSEGX(badvaddr) == CKSEG1) {
-#ifdef DEBUG
 		kvm_debug("Emulate Load from MMIO space @ %#lx\n", badvaddr);
-#endif
 		er = kvm_mips_emulate_inst(cause, opc, run, vcpu);
 		if (er == EMULATE_FAIL) {
 			printk("Emulate Load from MMIO space failed\n");
-- 
1.8.1.2

[PATCH 20/21] MIPS: KVM: Quieten kvm_info() logging

[James Hogan]

The logging from MIPS KVM is fairly noisy with kvm_info() in places
where it shouldn't be, such as on VM creation and migration to a
different CPU. Replace these kvm_info() calls with kvm_debug().

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_mips.c | 27 +++++++++++++--------------
 arch/mips/kvm/kvm_tlb.c  | 16 ++++++++--------
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 0cf03efe8a57..c0c4f20191b5 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -130,8 +130,8 @@ static void kvm_mips_init_vm_percpu(void *arg)
 int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 {
 	if (atomic_inc_return(&kvm_mips_instance) == 1) {
-		kvm_info("%s: 1st KVM instance, setup host TLB parameters\n",
-			 __func__);
+		kvm_debug("%s: 1st KVM instance, setup host TLB parameters\n",
+			  __func__);
 		on_each_cpu(kvm_mips_init_vm_percpu, kvm, 1);
 	}
 
@@ -186,8 +186,8 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 
 	/* If this is the last instance, restore wired count */
 	if (atomic_dec_return(&kvm_mips_instance) == 0) {
-		kvm_info("%s: last KVM instance, restoring TLB parameters\n",
-			 __func__);
+		kvm_debug("%s: last KVM instance, restoring TLB parameters\n",
+			  __func__);
 		on_each_cpu(kvm_mips_uninit_tlbs, NULL, 1);
 	}
 }
@@ -249,9 +249,8 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 				goto out;
 			}
 
-			kvm_info
-			    ("Allocated space for Guest PMAP Table (%ld pages) @ %p\n",
-			     npages, kvm->arch.guest_pmap);
+			kvm_debug("Allocated space for Guest PMAP Table (%ld pages) @ %p\n",
+				  npages, kvm->arch.guest_pmap);
 
 			/* Now setup the page table */
 			for (i = 0; i < npages; i++) {
@@ -296,7 +295,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 	if (err)
 		goto out_free_cpu;
 
-	kvm_info("kvm @ %p: create cpu %d at %p\n", kvm, id, vcpu);
+	kvm_debug("kvm @ %p: create cpu %d at %p\n", kvm, id, vcpu);
 
 	/* Allocate space for host mode exception handlers that handle
 	 * guest mode exits
@@ -316,8 +315,8 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 		err = -ENOMEM;
 		goto out_free_cpu;
 	}
-	kvm_info("Allocated %d bytes for KVM Exception Handlers @ %p\n",
-		 ALIGN(size, PAGE_SIZE), gebase);
+	kvm_debug("Allocated %d bytes for KVM Exception Handlers @ %p\n",
+		  ALIGN(size, PAGE_SIZE), gebase);
 
 	/* Save new ebase */
 	vcpu->arch.guest_ebase = gebase;
@@ -342,9 +341,9 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 
 	/* General handler, relocate to unmapped space for sanity's sake */
 	offset = 0x2000;
-	kvm_info("Installing KVM Exception handlers @ %p, %#x bytes\n",
-		 gebase + offset,
-		 mips32_GuestExceptionEnd - mips32_GuestException);
+	kvm_debug("Installing KVM Exception handlers @ %p, %#x bytes\n",
+		  gebase + offset,
+		  mips32_GuestExceptionEnd - mips32_GuestException);
 
 	memcpy(gebase + offset, mips32_GuestException,
 	       mips32_GuestExceptionEnd - mips32_GuestException);
@@ -361,7 +360,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 		goto out_free_gebase;
 	}
 
-	kvm_info("Allocated COMM page @ %p\n", vcpu->arch.kseg0_commpage);
+	kvm_debug("Allocated COMM page @ %p\n", vcpu->arch.kseg0_commpage);
 	kvm_mips_commpage_init(vcpu);
 
 	/* Init */
diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 51e7866ea893..0cdd586b0dc5 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -662,17 +662,17 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 		    vcpu->arch.guest_user_mm.context.asid[cpu];
 		newasid++;
 
-		kvm_info("[%d]: cpu_context: %#lx\n", cpu,
-			 cpu_context(cpu, current->mm));
-		kvm_info("[%d]: Allocated new ASID for Guest Kernel: %#x\n",
-			 cpu, vcpu->arch.guest_kernel_asid[cpu]);
-		kvm_info("[%d]: Allocated new ASID for Guest User: %#x\n", cpu,
-			 vcpu->arch.guest_user_asid[cpu]);
+		kvm_debug("[%d]: cpu_context: %#lx\n", cpu,
+			  cpu_context(cpu, current->mm));
+		kvm_debug("[%d]: Allocated new ASID for Guest Kernel: %#x\n",
+			  cpu, vcpu->arch.guest_kernel_asid[cpu]);
+		kvm_debug("[%d]: Allocated new ASID for Guest User: %#x\n", cpu,
+			  vcpu->arch.guest_user_asid[cpu]);
 	}
 
 	if (vcpu->arch.last_sched_cpu != cpu) {
-		kvm_info("[%d->%d]KVM VCPU[%d] switch\n",
-			 vcpu->arch.last_sched_cpu, cpu, vcpu->vcpu_id);
+		kvm_debug("[%d->%d]KVM VCPU[%d] switch\n",
+			  vcpu->arch.last_sched_cpu, cpu, vcpu->vcpu_id);
 		/*
 		 * Migrate the timer interrupt to the current CPU so that it
 		 * always interrupts the guest and synchronously triggers a
-- 
1.8.1.2

[PATCH 21/21] MIPS: KVM: Remove redundant NULL checks before kfree()

[James Hogan]

The kfree() function already NULL checks the parameter so remove the
redundant NULL checks before kfree() calls in arch/mips/kvm/.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: kvm@vger.kernel.org
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
 arch/mips/kvm/kvm_mips.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index c0c4f20191b5..094bc085eba6 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -149,9 +149,7 @@ void kvm_mips_free_vcpus(struct kvm *kvm)
 		if (kvm->arch.guest_pmap[i] != KVM_INVALID_PAGE)
 			kvm_mips_release_pfn_clean(kvm->arch.guest_pmap[i]);
 	}
-
-	if (kvm->arch.guest_pmap)
-		kfree(kvm->arch.guest_pmap);
+	kfree(kvm->arch.guest_pmap);
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
 		kvm_arch_vcpu_free(vcpu);
@@ -389,12 +387,8 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
 
 	kvm_mips_dump_stats(vcpu);
 
-	if (vcpu->arch.guest_ebase)
-		kfree(vcpu->arch.guest_ebase);
-
-	if (vcpu->arch.kseg0_commpage)
-		kfree(vcpu->arch.kseg0_commpage);
-
+	kfree(vcpu->arch.guest_ebase);
+	kfree(vcpu->arch.kseg0_commpage);
 }
 
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
-- 
1.8.1.2

Re: [PATCH 00/21] MIPS: KVM: Fixes and guest timer rewrite

[Paolo Bonzini]

Il 25/04/2014 17:19, James Hogan ha scritto:
> Here are a range of MIPS KVM T&E fixes for v3.16. They can also be found
> on my kvm_mips_queue branch here:
> git://git.kernel.org/pub/scm/linux/kernel/git/jhogan/kvm-mips.git
>
> They originally served to allow it to work better on Ingenic XBurst
> cores which have some peculiarities which break non-portable assumptions
> in the MIPS KVM implementation (patches 1-3, 11).
>
> Fixing guest CP0_Count emulation to work without a running host
> CP0_Count (patch 11) however required a rewrite of the timer emulation
> code to use the kernel monotonic time instead, which needed doing anyway
> since basing it directly off the host CP0_Count was broken. Various bugs
> were fixed in the process (patches 8-10) and improvements made thanks to
> valuable feedback from Paolo Bonzini for the last QEMU MIPS/KVM patchset
> (patches 4-7, 13-15).
>
> Finally there are some misc cleanups which I did along the way (patches
> 16-21).
>
> Only the first patch (fixes MIPS KVM with 4K pages) is marked for
> stable. For KVM to work on XBurst it needs the timer rework which is a
> fairly complex change, so there's little point marking any of the XBurst
> specific changes for stable.
>
> All feedback welcome!
>
> Patches 1-3:
> 	Fix KVM/MIPS with 4K pages, missing RDHWR SYNCI (XBurst),
> 	unmoving CP0_Random (XBurst).
> Patches 4-7:
> 	Add EPC, Count, Compare guest CP0 registers to KVM register
> 	ioctl interface.
> Patches 8-10:
> 	Fix a few potential races relating to timers.
> Patches 11-12:
> 	Rewrite guest timer emulation to use ktime_get().
> Patches 13-15:
> 	Add KVM virtual registers for controlling guest timer, including
> 	master timer disable, nanosecond bias, and timer frequency.
> Patches 16-21:
> 	Cleanups.
>
> James Hogan (21):
>   MIPS: KVM: Allocate at least 16KB for exception handlers
>   MIPS: KVM: Use local_flush_icache_range to fix RI on XBurst
>   MIPS: KVM: Use tlb_write_random
>   MIPS: KVM: Fix CP0_EBASE KVM register id
>   MIPS: KVM: Add CP0_EPC KVM register access
>   MIPS: KVM: Move KVM_{GET,SET}_ONE_REG definitions into kvm_host.h
>   MIPS: KVM: Add CP0_Count/Compare KVM register access
>   MIPS: KVM: Deliver guest interrupts after local_irq_disable()
>   MIPS: KVM: Fix timer race modifying guest CP0_Cause
>   MIPS: KVM: Migrate hrtimer to follow VCPU
>   MIPS: KVM: Rewrite count/compare timer emulation
>   MIPS: KVM: Override guest kernel timer frequency directly
>   MIPS: KVM: Add master disable count interface
>   MIPS: KVM: Add nanosecond count bias KVM register
>   MIPS: KVM: Add count frequency KVM register
>   MIPS: KVM: Make kvm_mips_comparecount_{func,wakeup} static
>   MIPS: KVM: Whitespace fixes in kvm_mips_callbacks
>   MIPS: KVM: Fix kvm_debug bit-rottage
>   MIPS: KVM: Remove ifdef DEBUG around kvm_debug
>   MIPS: KVM: Quieten kvm_info() logging
>   MIPS: KVM: Remove redundant NULL checks before kfree()
>
>  arch/mips/Kconfig                 |  12 +-
>  arch/mips/include/asm/kvm_host.h  | 185 +++++++++---
>  arch/mips/include/uapi/asm/kvm.h  |  40 +++
>  arch/mips/kvm/kvm_locore.S        |  32 --
>  arch/mips/kvm/kvm_mips.c          | 127 ++++----
>  arch/mips/kvm/kvm_mips_dyntrans.c |  15 +-
>  arch/mips/kvm/kvm_mips_emul.c     | 608 ++++++++++++++++++++++++++++++++++++--
>  arch/mips/kvm/kvm_tlb.c           |  60 ++--
>  arch/mips/kvm/kvm_trap_emul.c     |  89 +++++-
>  arch/mips/mti-malta/malta-time.c  |  14 +-
>  10 files changed, 951 insertions(+), 231 deletions(-)
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: kvm@vger.kernel.org
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: David Daney <david.daney@cavium.com>
> Cc: Sanjay Lal <sanjayl@kymasys.com>
>

There weren't too many comments on this series, and it would be really 
nice to have it in 3.16.

Thanks,

Paolo