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

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

[James Hogan]

Here are a range of MIPS KVM T&E fixes, preferably for v3.16 but I know
it's probably a bit late now. Changes are pretty minimal though since
v1 so please consider. They can also be found on my kvm_mips_queue
branch (and the kvm_mips_timer_v2 tag) 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-4, 13).

Fixing guest CP0_Count emulation to work without a running host
CP0_Count (patch 13) 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 10-12) and improvements made thanks to
valuable feedback from Paolo Bonzini for the last QEMU MIPS/KVM patchset
(patches 5-7, 15-16).

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

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-4:
	Fix KVM/MIPS with 4K pages, missing RDHWR SYNCI (XBurst),
	unmoving CP0_Random (XBurst).
Patches 5-9:
	Add EPC, Count, Compare, UserLocal, HWREna guest CP0 registers
	to KVM register ioctl interface.
Patches 10-12:
	Fix a few potential races relating to timers.
Patches 13-14:
	Rewrite guest timer emulation to use ktime_get().
Patches 15-16:
	Add KVM virtual registers for controlling guest timer, including
	master timer disable, and timer frequency.
Patches 17-23:
	Cleanups.

Changes in v2 (tag:kvm_mips_timer_v2):
 Patchset:
 - Drop patch 4 "MIPS: KVM: Fix CP0_EBASE KVM register id" (David
   Daney).
 - Drop patch 14 "MIPS: KVM: Add nanosecond count bias KVM register".
   The COUNT_CTL and COUNT_RESUME API is clean and sufficient.
 - Add missing access to UserLocal and HWREna guest CP0 registers
   (patches 15 and 16).
 - Add export of local_flush_icache_range (patch 2).
 Patch 12 MIPS: KVM: Migrate hrtimer to follow VCPU
 - Move kvm_mips_migrate_count() into kvm_tlb.c to fix a link error when
   KVM is built as a module, since kvm_tlb.c is built statically and
   cannot reference symbols in kvm_mips_emul.c.
 Patch 15 MIPS: KVM: Add master disable count interface
 - Make KVM_REG_MIPS_COUNT_RESUME writable too so that userland can
   control timer using master DC and without bias register. New values
   are rejected if they refer to a monotonic time in the future.
 - Expand on description of KVM_REG_MIPS_COUNT_RESUME about the effects
   of the register and that it can be written.

v1 (tag:kvm_mips_timer_v1):
 see http://marc.info/?l=kvm&m=139843936102657&w=2

James Hogan (23):
  MIPS: KVM: Allocate at least 16KB for exception handlers
  MIPS: Export local_flush_icache_range for KVM
  MIPS: KVM: Use local_flush_icache_range to fix RI on XBurst
  MIPS: KVM: Use tlb_write_random
  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: Add CP0_UserLocal KVM register access
  MIPS: KVM: Add CP0_HWREna 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 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()
  MIPS: KVM: Remove redundant semicolon

 arch/mips/Kconfig                 |  12 +-
 arch/mips/include/asm/kvm_host.h  | 183 ++++++++++---
 arch/mips/include/uapi/asm/kvm.h  |  35 +++
 arch/mips/kvm/kvm_locore.S        |  32 ---
 arch/mips/kvm/kvm_mips.c          | 140 +++++-----
 arch/mips/kvm/kvm_mips_dyntrans.c |  15 +-
 arch/mips/kvm/kvm_mips_emul.c     | 557 ++++++++++++++++++++++++++++++++++++--
 arch/mips/kvm/kvm_tlb.c           |  77 +++---
 arch/mips/kvm/kvm_trap_emul.c     |  86 +++++-
 arch/mips/mm/cache.c              |   1 +
 arch/mips/mti-malta/malta-time.c  |  14 +-
 11 files changed, 920 insertions(+), 232 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: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
-- 
1.9.3

[PATCH v2 01/23] 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.9.3

[PATCH v2 02/23] MIPS: Export local_flush_icache_range for KVM

[James Hogan]

Export the local_flush_icache_range function pointer for GPL modules so
that it can be used by KVM for syncing the icache after binary
translation of trapping instructions.

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>
---
v2:
 - New patch.
---
 arch/mips/mm/cache.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
index 9e67cdea3c74..f7b91d3a371d 100644
--- a/arch/mips/mm/cache.c
+++ b/arch/mips/mm/cache.c
@@ -31,6 +31,7 @@ void (*flush_cache_page)(struct vm_area_struct *vma, unsigned long page,
 void (*flush_icache_range)(unsigned long start, unsigned long end);
 EXPORT_SYMBOL_GPL(flush_icache_range);
 void (*local_flush_icache_range)(unsigned long start, unsigned long end);
+EXPORT_SYMBOL_GPL(local_flush_icache_range);
 
 void (*__flush_cache_vmap)(void);
 void (*__flush_cache_vunmap)(void);
-- 
1.9.3

[PATCH v2 03/23] 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.9.3

[PATCH v2 04/23] 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.9.3

[PATCH v2 05/23] 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 14511138f187..85d5552f0a47 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_64(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.9.3

[PATCH v2 06/23] 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..9f6bfc963a5b 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_64(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 85d5552f0a47..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_64(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.9.3

[PATCH v2 07/23] 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 9f6bfc963a5b..41e180ed36e3 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.9.3

[PATCH v2 08/23] MIPS: KVM: Add CP0_UserLocal KVM register access

[James Hogan]

Implement KVM_{GET,SET}_ONE_REG ioctl based access to the guest CP0
UserLocal register. This is so that userland can save and restore its
value.

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>
---
v2:
 - New patch.
---
 arch/mips/include/asm/kvm_host.h | 1 +
 arch/mips/kvm/kvm_mips.c         | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 41e180ed36e3..6f9338450e24 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -442,6 +442,7 @@ struct kvm_vcpu_arch {
 #define kvm_read_c0_guest_context(cop0)		(cop0->reg[MIPS_CP0_TLB_CONTEXT][0])
 #define kvm_write_c0_guest_context(cop0, val)	(cop0->reg[MIPS_CP0_TLB_CONTEXT][0] = (val))
 #define kvm_read_c0_guest_userlocal(cop0)	(cop0->reg[MIPS_CP0_TLB_CONTEXT][2])
+#define kvm_write_c0_guest_userlocal(cop0, val)	(cop0->reg[MIPS_CP0_TLB_CONTEXT][2] = (val))
 #define kvm_read_c0_guest_pagemask(cop0)	(cop0->reg[MIPS_CP0_TLB_PG_MASK][0])
 #define kvm_write_c0_guest_pagemask(cop0, val)	(cop0->reg[MIPS_CP0_TLB_PG_MASK][0] = (val))
 #define kvm_read_c0_guest_wired(cop0)		(cop0->reg[MIPS_CP0_TLB_WIRED][0])
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 2f6344cca653..153c1e17afb1 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -531,6 +531,7 @@ static u64 kvm_mips_get_one_regs[] = {
 
 	KVM_REG_MIPS_CP0_INDEX,
 	KVM_REG_MIPS_CP0_CONTEXT,
+	KVM_REG_MIPS_CP0_USERLOCAL,
 	KVM_REG_MIPS_CP0_PAGEMASK,
 	KVM_REG_MIPS_CP0_WIRED,
 	KVM_REG_MIPS_CP0_BADVADDR,
@@ -575,6 +576,9 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_CONTEXT:
 		v = (long)kvm_read_c0_guest_context(cop0);
 		break;
+	case KVM_REG_MIPS_CP0_USERLOCAL:
+		v = (long)kvm_read_c0_guest_userlocal(cop0);
+		break;
 	case KVM_REG_MIPS_CP0_PAGEMASK:
 		v = (long)kvm_read_c0_guest_pagemask(cop0);
 		break;
@@ -683,6 +687,9 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_CONTEXT:
 		kvm_write_c0_guest_context(cop0, v);
 		break;
+	case KVM_REG_MIPS_CP0_USERLOCAL:
+		kvm_write_c0_guest_userlocal(cop0, v);
+		break;
 	case KVM_REG_MIPS_CP0_PAGEMASK:
 		kvm_write_c0_guest_pagemask(cop0, v);
 		break;
-- 
1.9.3

[PATCH v2 09/23] MIPS: KVM: Add CP0_HWREna KVM register access

[James Hogan]

Implement KVM_{GET,SET}_ONE_REG ioctl based access to the guest CP0
HWREna register. This is so that userland can save and restore its
value so that RDHWR instructions don't have to be emulated by the guest.

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>
---
v2:
 - New patch.
---
 arch/mips/kvm/kvm_mips.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 153c1e17afb1..73f67fc92b7a 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -534,6 +534,7 @@ static u64 kvm_mips_get_one_regs[] = {
 	KVM_REG_MIPS_CP0_USERLOCAL,
 	KVM_REG_MIPS_CP0_PAGEMASK,
 	KVM_REG_MIPS_CP0_WIRED,
+	KVM_REG_MIPS_CP0_HWRENA,
 	KVM_REG_MIPS_CP0_BADVADDR,
 	KVM_REG_MIPS_CP0_COUNT,
 	KVM_REG_MIPS_CP0_ENTRYHI,
@@ -585,6 +586,9 @@ static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_WIRED:
 		v = (long)kvm_read_c0_guest_wired(cop0);
 		break;
+	case KVM_REG_MIPS_CP0_HWRENA:
+		v = (long)kvm_read_c0_guest_hwrena(cop0);
+		break;
 	case KVM_REG_MIPS_CP0_BADVADDR:
 		v = (long)kvm_read_c0_guest_badvaddr(cop0);
 		break;
@@ -696,6 +700,9 @@ static int kvm_mips_set_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_CP0_WIRED:
 		kvm_write_c0_guest_wired(cop0, v);
 		break;
+	case KVM_REG_MIPS_CP0_HWRENA:
+		kvm_write_c0_guest_hwrena(cop0, v);
+		break;
 	case KVM_REG_MIPS_CP0_BADVADDR:
 		kvm_write_c0_guest_badvaddr(cop0, v);
 		break;
-- 
1.9.3

[PATCH v2 10/23] 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 73f67fc92b7a..2ca84e2efcea 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.9.3

[PATCH v2 11/23] 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 6f9338450e24..79410f85a5a7 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -482,15 +482,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.9.3

[PATCH v2 12/23] 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>
---
v2:
 - Move kvm_mips_migrate_count() into kvm_tlb.c to fix a link error when
   KVM is built as a module, since kvm_tlb.c is built statically and
   cannot reference symbols in kvm_mips_emul.c.
---
 arch/mips/kvm/kvm_tlb.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 9d371ee0a755..d65999a9f8af 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -656,6 +656,23 @@ void kvm_local_flush_tlb_all(void)
 	local_irq_restore(flags);
 }
 
+/**
+ * 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.
+ */
+static void kvm_mips_migrate_count(struct kvm_vcpu *vcpu)
+{
+	if (hrtimer_cancel(&vcpu->arch.comparecount_timer))
+		hrtimer_restart(&vcpu->arch.comparecount_timer);
+}
+
 /* Restore ASID once we are scheduled back after preemption */
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
@@ -691,6 +708,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.9.3

[PATCH v2 13/23] 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    | 393 ++++++++++++++++++++++++++++++++++++---
 arch/mips/kvm/kvm_trap_emul.c    |  27 ++-
 4 files changed, 413 insertions(+), 38 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 79410f85a5a7..75ed94aeefe7 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 */
@@ -705,7 +710,13 @@ 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_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 2ca84e2efcea..a5f33cbc13ab 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;
 
@@ -712,9 +712,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;
@@ -724,6 +721,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;
@@ -997,9 +995,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 bad31c6235d4..088c25d73a11 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,364 @@ 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;
+	}
+
+	/*
+	 * 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 er;
+	/* 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;
+
+	/* Calculate bias */
+	now = ktime_get();
+	vcpu->arch.count_bias = count - kvm_mips_ktime_to_count(vcpu, now);
+
+	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);
+}
+
+/**
+ * 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)
@@ -471,8 +811,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
@@ -539,10 +878,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",
@@ -552,8 +888,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]);
@@ -564,6 +900,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
@@ -1553,8 +1903,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.9.3

[PATCH v2 14/23] 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.9.3

[PATCH v2 15/23] 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>
---
v2:
 - Make KVM_REG_MIPS_COUNT_RESUME writable too so that userland can
   control timer using master DC and without bias register. New values
   are rejected if they refer to a monotonic time in the future.
 - Expand on description of KVM_REG_MIPS_COUNT_RESUME about the effects
   of the register and that it can be written.
---
 arch/mips/include/asm/kvm_host.h |   6 ++
 arch/mips/include/uapi/asm/kvm.h |  28 +++++++++
 arch/mips/kvm/kvm_mips.c         |   9 ++-
 arch/mips/kvm/kvm_mips_emul.c    | 132 +++++++++++++++++++++++++++++++++++----
 arch/mips/kvm/kvm_trap_emul.c    |  15 ++++-
 5 files changed, 176 insertions(+), 14 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 75ed94aeefe7..1deeaecbe73e 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;
 
@@ -714,6 +718,8 @@ 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);
+int kvm_mips_set_count_resume(struct kvm_vcpu *vcpu, s64 count_resume);
 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);
diff --git a/arch/mips/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
index f09ff5ae2059..f859fbada1f7 100644
--- a/arch/mips/include/uapi/asm/kvm.h
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -106,6 +106,34 @@ 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). Any reads and writes of Count related registers while
+ * COUNT_CTL.DC=1 will appear to occur at this time. When COUNT_CTL.DC is
+ * cleared again (master enable) any timer interrupts since this time will be
+ * emulated.
+ * Modifications to times in the future are rejected.
+ */
+#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 a5f33cbc13ab..216ecaf2c18a 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -547,7 +547,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,
@@ -627,6 +630,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;
@@ -722,6 +727,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 088c25d73a11..65c8dea6d1f5 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.
@@ -448,7 +467,7 @@ void kvm_mips_write_count(struct kvm_vcpu *vcpu, uint32_t count)
 	ktime_t now;
 
 	/* Calculate bias */
-	now = ktime_get();
+	now = kvm_mips_count_time(vcpu);
 	vcpu->arch.count_bias = count - kvm_mips_ktime_to_count(vcpu, now);
 
 	if (kvm_mips_count_disabled(vcpu))
@@ -508,8 +527,8 @@ void kvm_mips_write_compare(struct kvm_vcpu *vcpu, uint32_t compare)
  * 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.
  */
@@ -535,7 +554,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).
  */
@@ -544,7 +564,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);
 }
 
 /**
@@ -552,9 +573,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).
  */
@@ -567,13 +588,100 @@ 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_set_count_resume() - Update the count resume KVM register.
+ * @vcpu:		Virtual CPU.
+ * @count_resume:	Count resume register new value.
+ *
+ * Set the count resume KVM register.
+ *
+ * Returns:	-EINVAL if out of valid range (0..now).
+ *		0 on success.
+ */
+int kvm_mips_set_count_resume(struct kvm_vcpu *vcpu, s64 count_resume)
+{
+	/*
+	 * It doesn't make sense for the resume time to be in the future, as it
+	 * would be possible for the next interrupt to be more than a full
+	 * period in the future.
+	 */
+	if (count_resume < 0 || count_resume > ktime_to_ns(ktime_get()))
+		return -EINVAL;
+
+	vcpu->arch.count_resume = ns_to_ktime(count_resume);
+	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..854502bcc749 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,16 @@ 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;
+	case KVM_REG_MIPS_COUNT_RESUME:
+		ret = kvm_mips_set_count_resume(vcpu, v);
+		break;
 	default:
 		return -EINVAL;
 	}
-	return 0;
+	return ret;
 }
 
 static struct kvm_mips_callbacks kvm_trap_emul_callbacks = {
-- 
1.9.3

[PATCH v2 16/23] 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    | 48 ++++++++++++++++++++++++++++++++++++++++
 arch/mips/kvm/kvm_trap_emul.c    |  6 +++++
 5 files changed, 65 insertions(+)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 1deeaecbe73e..f9c672f729ea 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -720,6 +720,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);
 int kvm_mips_set_count_resume(struct kvm_vcpu *vcpu, s64 count_resume);
+int kvm_mips_set_count_hz(struct kvm_vcpu *vcpu, s64 count_hz);
 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);
diff --git a/arch/mips/include/uapi/asm/kvm.h b/arch/mips/include/uapi/asm/kvm.h
index f859fbada1f7..2c04b6d9ff85 100644
--- a/arch/mips/include/uapi/asm/kvm.h
+++ b/arch/mips/include/uapi/asm/kvm.h
@@ -133,6 +133,13 @@ struct kvm_fpu {
  */
 #define KVM_REG_MIPS_COUNT_RESUME	(KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
 					 0x20000 | 1)
+/*
+ * 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 | 2)
 
 /*
  * KVM MIPS specific structures and definitions
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 216ecaf2c18a..1472b0b3d93b 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -551,6 +551,7 @@ static u64 kvm_mips_get_one_regs[] = {
 
 	KVM_REG_MIPS_COUNT_CTL,
 	KVM_REG_MIPS_COUNT_RESUME,
+	KVM_REG_MIPS_COUNT_HZ,
 };
 
 static int kvm_mips_get_reg(struct kvm_vcpu *vcpu,
@@ -632,6 +633,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_HZ:
 		ret = kvm_mips_callbacks->get_one_reg(vcpu, reg, &v);
 		if (ret)
 			return ret;
@@ -729,6 +731,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_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 65c8dea6d1f5..6d257384c9b4 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -498,6 +498,54 @@ void kvm_mips_init_count(struct kvm_vcpu *vcpu)
 }
 
 /**
+ * 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;
+
+	/* 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 */
+	vcpu->arch.count_bias = count - kvm_mips_ktime_to_count(vcpu, now);
+
+	/* 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 854502bcc749..b171db324cf0 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_HZ:
+		*v = vcpu->arch.count_hz;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -461,6 +464,9 @@ static int kvm_trap_emul_set_one_reg(struct kvm_vcpu *vcpu,
 	case KVM_REG_MIPS_COUNT_RESUME:
 		ret = kvm_mips_set_count_resume(vcpu, v);
 		break;
+	case KVM_REG_MIPS_COUNT_HZ:
+		ret = kvm_mips_set_count_hz(vcpu, v);
+		break;
 	default:
 		return -EINVAL;
 	}
-- 
1.9.3

[PATCH v2 17/23] 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 1472b0b3d93b..a4d6cf96261a 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -984,7 +984,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;
 
@@ -999,7 +999,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.9.3

[PATCH v2 18/23] 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 f9c672f729ea..b0aa95565752 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -569,29 +569,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.9.3

[PATCH v2 19/23] 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 6d257384c9b4..c81ab791b8f2 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -1831,8 +1831,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 d65999a9f8af..994fc2384180 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.9.3

[PATCH v2 20/23] 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 c81ab791b8f2..8d4840090082 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -2319,11 +2319,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 994fc2384180..15ad06d717fd 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;
 }
@@ -675,9 +663,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 b171db324cf0..693f952b2fbb 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.9.3

[PATCH v2 21/23] 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 a4d6cf96261a..687be9ca1f74 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 15ad06d717fd..8a5a700ad8de 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -679,17 +679,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.9.3

[PATCH v2 22/23] 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 687be9ca1f74..1a4330847153 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.9.3

[PATCH v2 23/23] MIPS: KVM: Remove redundant semicolon

[James Hogan]

Remove extra semicolon in kvm_arch_vcpu_dump_regs().

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>
---
v2:
 - New patch.
---
 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 1a4330847153..fd858a9e1f8a 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -928,7 +928,7 @@ int kvm_arch_vcpu_dump_regs(struct kvm_vcpu *vcpu)
 		return -1;
 
 	printk("VCPU Register Dump:\n");
-	printk("\tpc = 0x%08lx\n", vcpu->arch.pc);;
+	printk("\tpc = 0x%08lx\n", vcpu->arch.pc);
 	printk("\texceptions: %08lx\n", vcpu->arch.pending_exceptions);
 
 	for (i = 0; i < 32; i += 4) {
-- 
1.9.3

Re: [PATCH v2 11/23] MIPS: KVM: Fix timer race modifying guest CP0_Cause

[Paolo Bonzini]

Il 29/05/2014 11:16, James Hogan ha scritto:
> 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.

Shouldn't you have a loop too around the ll/sc?

Paolo

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

[Paolo Bonzini]

Il 29/05/2014 11:16, James Hogan ha scritto:
> Here are a range of MIPS KVM T&E fixes, preferably for v3.16 but I know
> it's probably a bit late now. Changes are pretty minimal though since
> v1 so please consider. They can also be found on my kvm_mips_queue
> branch (and the kvm_mips_timer_v2 tag) here:
> git://git.kernel.org/pub/scm/linux/kernel/git/jhogan/kvm-mips.git

That's okay for me, but I'd like to get a look at the QEMU parts.

I would also like an Acked-by for patches 2 and 14, and I have a 
question about patch 11.

Paolo

Re: [PATCH v2 11/23] MIPS: KVM: Fix timer race modifying guest CP0_Cause

[James Hogan]

Hi Paolo,

On 29/05/14 11:36, Paolo Bonzini wrote:
> Il 29/05/2014 11:16, James Hogan ha scritto:
>> 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.
> 
> Shouldn't you have a loop too around the ll/sc?

Yes, it has a do {} while () look around the inline asm, although I
didn't mention it in the commit message. It's modelled on
arch/mips/include/asm/bitops.h.

Cheers
James

Re: [PATCH v2 11/23] MIPS: KVM: Fix timer race modifying guest CP0_Cause

[Paolo Bonzini]

Il 29/05/2014 12:55, James Hogan ha scritto:
>> > Shouldn't you have a loop too around the ll/sc?
> Yes, it has a do {} while () look around the inline asm, although I
> didn't mention it in the commit message. It's modelled on
> arch/mips/include/asm/bitops.h.

Ugh, sorry---I misread that as a do/while(0).  But it is an inline 
function, not a macro.

Paolo

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

[James Hogan]

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

On 29/05/14 11:36, Paolo Bonzini wrote:
> Il 29/05/2014 11:16, James Hogan ha scritto:
>> Here are a range of MIPS KVM T&E fixes, preferably for v3.16 but I know
>> it's probably a bit late now. Changes are pretty minimal though since
>> v1 so please consider. They can also be found on my kvm_mips_queue
>> branch (and the kvm_mips_timer_v2 tag) here:
>> git://git.kernel.org/pub/scm/linux/kernel/git/jhogan/kvm-mips.git
> 
> That's okay for me, but I'd like to get a look at the QEMU parts.

I'm still in the process of cleaning up my qemu patchset, but the
attached 2 patches hopefully shows the gist of it this particular change.

Thanks
James

> 
> I would also like an Acked-by for patches 2 and 14, and I have a
> question about patch 11.
> 
> Paolo

[-- Attachment #2: 0001-timer-Add-cpu_get_clock_at.patch --]
[-- Type: text/x-patch, Size: 2125 bytes --]

>From 1438f5eb5e05eb5ef8b90a6d52cfe73e67da254f Mon Sep 17 00:00:00 2001
From: James Hogan <james.hogan@imgtec.com>
Date: Tue, 20 May 2014 00:52:08 +0100
Subject: [PATCH 1/2] timer: Add cpu_get_clock_at()

Add a new cpu_get_clock_at() which gets the VM time at a particular
specified monotonic time rather than the current monotonic time. This is
to allow for operations that may need both the current monotonic time
and the VM time based on that, such as to allow synchronisation with a
KVM CPU clock.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
 cpus.c               | 23 +++++++++++++++++++++--
 include/qemu/timer.h |  1 +
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index dd7ac13..ac906a3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -186,18 +186,37 @@ int64_t cpu_get_ticks(void)
     return ticks;
 }
 
-static int64_t cpu_get_clock_locked(void)
+static int64_t cpu_get_clock_at_locked(int64_t now)
 {
     int64_t ticks;
 
     ticks = timers_state.cpu_clock_offset;
     if (timers_state.cpu_ticks_enabled) {
-        ticks += get_clock();
+        ticks += now;
     }
 
     return ticks;
 }
 
+static int64_t cpu_get_clock_locked(void)
+{
+    return cpu_get_clock_at_locked(get_clock());
+}
+
+/* return the host CPU monotonic timer and handle stop/restart */
+int64_t cpu_get_clock_at(int64_t now)
+{
+    int64_t ti;
+    unsigned start;
+
+    do {
+        start = seqlock_read_begin(&timers_state.vm_clock_seqlock);
+        ti = cpu_get_clock_at_locked(now);
+    } while (seqlock_read_retry(&timers_state.vm_clock_seqlock, start));
+
+    return ti;
+}
+
 /* return the host CPU monotonic timer and handle stop/restart */
 int64_t cpu_get_clock(void)
 {
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 7f9a074..5b2e8c2 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -745,6 +745,7 @@ static inline int64_t get_clock(void)
 /* icount */
 int64_t cpu_get_icount(void);
 int64_t cpu_get_clock(void);
+int64_t cpu_get_clock_at(int64_t now);
 
 /*******************************************/
 /* host CPU ticks (if available) */
-- 
1.9.3


[-- Attachment #3: 0002-target-mips-kvm-Save-restore-KVM-timer-state.patch --]
[-- Type: text/x-patch, Size: 11053 bytes --]

>From 22c61769ec31cb222c5291c88bd6a658e23862f5 Mon Sep 17 00:00:00 2001
From: James Hogan <james.hogan@imgtec.com>
Date: Thu, 29 May 2014 14:36:20 +0100
Subject: [PATCH 2/2] target-mips: kvm: Save/restore KVM timer state

Save and restore KVM timer state properly. When it's saved (or VM clock
stopped) the VM clock is also recorded. When it's restored (or VM clock
restarted) it is resumed with the stored count at the saved VM clock
translated into monotonic time, i.e. current time - elapsed VM
nanoseconds. This therefore behaves correctly after the VM clock has
been stopped.

E.g.
sync state to QEMU
    takes snapshot of Count, Cause and VM time(now)
sync state back to KVM
    restores Count, Cause at translated VM time (unchanged)
    time continues from the same Count without losing time or interrupts

Or:
stop vm clock
    takes snapshot of Count, Cause and VM time

restart vm clock
    restores Count, Cause at now - (vm time(now) - saved vm time)
    time continues from the same Count but at a later monotonic time
    depending on how long the vm clock was stopped

The VM time at which the timer was stopped (count_save_time) is
saved/loaded by cpu_save and cpu_load so that it can be restarted
without losing time relative to the VM clock.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
 target-mips/cpu.h     |   3 +-
 target-mips/kvm.c     | 196 +++++++++++++++++++++++++++++++++++++++++++++++---
 target-mips/machine.c |   6 ++
 3 files changed, 195 insertions(+), 10 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 67bf441..02a8da1 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -495,6 +495,7 @@ struct CPUMIPSState {
     const mips_def_t *cpu_model;
     void *irq[8];
     QEMUTimer *timer; /* Internal timer */
+    int64_t count_save_time;
 };
 
 #include "cpu-qom.h"
@@ -526,7 +527,7 @@ void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf);
 extern void cpu_wrdsp(uint32_t rs, uint32_t mask_num, CPUMIPSState *env);
 extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
 
-#define CPU_SAVE_VERSION 4
+#define CPU_SAVE_VERSION 5
 
 /* MMU modes definitions. We carefully match the indices with our
    hflags layout. */
diff --git a/target-mips/kvm.c b/target-mips/kvm.c
index 8765205..ee2dd1c 100644
--- a/target-mips/kvm.c
+++ b/target-mips/kvm.c
@@ -33,6 +33,8 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_LAST_INFO
 };
 
+static void kvm_mips_update_state(void *opaque, int running, RunState state);
+
 unsigned long kvm_arch_vcpu_id(CPUState *cs)
 {
     return cs->cpu_index;
@@ -50,6 +52,9 @@ int kvm_arch_init(KVMState *s)
 int kvm_arch_init_vcpu(CPUState *cs)
 {
     int ret = 0;
+
+    qemu_add_vm_change_state_handler(kvm_mips_update_state, cs);
+
     DPRINTF("%s\n", __func__);
     return ret;
 }
@@ -224,6 +229,17 @@ int kvm_mips_set_ipi_interrupt(MIPSCPU *cpu, int irq, int level)
 #define KVM_REG_MIPS_CP0_XCONTEXT	MIPS_CP0_64(20, 0)
 #define KVM_REG_MIPS_CP0_ERROREPC	MIPS_CP0_64(30, 0)
 
+/* CP0_Count control */
+#define KVM_REG_MIPS_COUNT_CTL          (KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
+                                         0x20000 | 0)
+#define KVM_REG_MIPS_COUNT_CTL_DC       0x00000001      /* master disable */
+/* CP0_Count resume monotonic nanoseconds */
+#define KVM_REG_MIPS_COUNT_RESUME       (KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
+                                         0x20000 | 1)
+/* CP0_Count rate in Hz */
+#define KVM_REG_MIPS_COUNT_HZ           (KVM_REG_MIPS | KVM_REG_SIZE_U64 | \
+                                         0x20000 | 2)
+
 static inline int kvm_mips_put_one_reg(CPUState *cs, uint64_t reg_id,
                                        int32_t *addr)
 {
@@ -248,6 +264,17 @@ static inline int kvm_mips_put_one_ulreg(CPUState *cs, uint64_t reg_id,
     return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &cp0reg);
 }
 
+static inline int kvm_mips_put_one_reg64(CPUState *cs, uint64_t reg_id,
+                                         uint64_t *addr)
+{
+    struct kvm_one_reg cp0reg = {
+        .id = reg_id,
+        .addr = (uintptr_t)addr
+    };
+
+    return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &cp0reg);
+}
+
 static inline int kvm_mips_get_one_reg(CPUState *cs, uint64_t reg_id,
                                        int32_t *addr)
 {
@@ -286,6 +313,151 @@ static inline int kvm_mips_get_one_ulreg(CPUState *cs, uint64 reg_id,
     return ret;
 }
 
+static inline int kvm_mips_get_one_reg64(CPUState *cs, uint64 reg_id,
+                                         uint64_t *addr)
+{
+    int ret;
+    struct kvm_one_reg cp0reg = {
+        .id = reg_id,
+        .addr = (uintptr_t)addr
+    };
+
+    ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &cp0reg);
+    return ret;
+}
+
+/*
+ * We freeze the KVM timer when either the VM clock is stopped or the state is
+ * saved (the state is dirty).
+ */
+
+/*
+ * Save the state of the KVM timer when VM clock is stopped or state is synced
+ * to QEMU.
+ */
+static int kvm_mips_save_count(CPUState *cs)
+{
+    MIPSCPU *cpu = MIPS_CPU(cs);
+    CPUMIPSState *env = &cpu->env;
+    uint64_t count_ctl, count_resume;
+    int ret;
+
+    /* freeze KVM timer */
+    ret = kvm_mips_get_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, &count_ctl);
+    if (ret < 0) {
+        return ret;
+    }
+    if (!(count_ctl & KVM_REG_MIPS_COUNT_CTL_DC)) {
+        count_ctl |= KVM_REG_MIPS_COUNT_CTL_DC;
+        ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, &count_ctl);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    /* read CP0_Cause */
+    ret = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CAUSE, &env->CP0_Cause);
+    if (ret < 0) {
+        return ret;
+    }
+
+    /* read CP0_Count */
+    ret = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_COUNT, &env->CP0_Count);
+    if (ret < 0) {
+        return ret;
+    }
+
+    /* read COUNT_RESUME */
+    ret = kvm_mips_get_one_reg64(cs, KVM_REG_MIPS_COUNT_RESUME,
+                                 &count_resume);
+    if (ret < 0) {
+        return ret;
+    }
+
+    /* translate monotonic COUNT_RESUME to VM time */
+    env->count_save_time = cpu_get_clock_at(count_resume);
+
+    return ret;
+}
+
+/*
+ * Restore the state of the KVM timer when VM clock is restarted or state is
+ * synced to KVM.
+ */
+static int kvm_mips_restore_count(CPUState *cs)
+{
+    MIPSCPU *cpu = MIPS_CPU(cs);
+    CPUMIPSState *env = &cpu->env;
+    uint64_t count_ctl, count_resume;
+    int64_t now = get_clock();
+    int ret = 0;
+
+    /* check the timer is frozen */
+    ret = kvm_mips_get_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, &count_ctl);
+    if (ret < 0) {
+        return ret;
+    }
+    if (!(count_ctl & KVM_REG_MIPS_COUNT_CTL_DC)) {
+        /* freeze timer (sets COUNT_RESUME for us) */
+        count_ctl |= KVM_REG_MIPS_COUNT_CTL_DC;
+        ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, &count_ctl);
+        if (ret < 0) {
+            return ret;
+        }
+    } else {
+        /* find time to resume the saved timer at */
+        now = get_clock();
+        count_resume = now - (cpu_get_clock_at(now) - env->count_save_time);
+        ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_RESUME,
+                                     &count_resume);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    /* load CP0_Cause */
+    ret = kvm_mips_put_one_reg(cs, KVM_REG_MIPS_CP0_CAUSE, &env->CP0_Cause);
+    if (ret < 0) {
+        return ret;
+    }
+
+    /* load CP0_Count */
+    ret = kvm_mips_put_one_reg(cs, KVM_REG_MIPS_CP0_COUNT, &env->CP0_Count);
+    if (ret < 0) {
+        return ret;
+    }
+
+    /* resume KVM timer */
+    count_ctl &= ~KVM_REG_MIPS_COUNT_CTL_DC;
+    ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, &count_ctl);
+    return ret;
+}
+
+/*
+ * Handle the VM clock being started or stopped
+ */
+static void kvm_mips_update_state(void *opaque, int running, RunState state)
+{
+    CPUState *cs = opaque;
+    int ret;
+
+    /*
+     * If state is already dirty (synced to QEMU) then the KVM timer state is
+     * already saved and can be restored when it is synced back to KVM.
+     */
+    if (!cs->kvm_vcpu_dirty) {
+        if (running) {
+            ret = kvm_mips_restore_count(cs);
+        } else {
+            ret = kvm_mips_save_count(cs);
+        }
+        if (ret < 0) {
+            fprintf(stderr, "Failed update count (running=%d)\n",
+                    running);
+        }
+    }
+}
+
 static int kvm_mips_put_cp0_registers(CPUState *cs, int level)
 {
     MIPSCPU *cpu = MIPS_CPU(cs);
@@ -322,15 +494,16 @@ static int kvm_mips_put_cp0_registers(CPUState *cs, int level)
         fprintf(stderr, "Failed to put BADVADDR\n");
         return ret;
     }
-    if (level != KVM_PUT_RUNTIME_STATE) {
-        /* don't write guest count during runtime or the clock might drift */
-        ret |= kvm_mips_put_one_reg(cs, KVM_REG_MIPS_CP0_COUNT,
-                                    &env->CP0_Count);
+
+    /* If VM clock stopped then state will be restored when it is restarted */
+    if (runstate_is_running()) {
+        ret = kvm_mips_restore_count(cs);
         if (ret < 0) {
-            fprintf(stderr, "Failed to put COMPARE\n");
+            fprintf(stderr, "Failed to put COUNT\n");
             return ret;
         }
     }
+
     ret |= kvm_mips_put_one_ulreg(cs, KVM_REG_MIPS_CP0_ENTRYHI,
                                   &env->CP0_EntryHi);
     if (ret < 0) {
@@ -397,10 +570,6 @@ static int kvm_mips_get_cp0_registers(CPUState *cs)
     if (ret < 0) {
         return ret;
     }
-    ret |= kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_COUNT, &env->CP0_Count);
-    if (ret < 0) {
-        return ret;
-    }
     ret |= kvm_mips_get_one_ulreg(cs, KVM_REG_MIPS_CP0_ENTRYHI,
                                   &env->CP0_EntryHi);
     if (ret < 0) {
@@ -419,6 +588,15 @@ static int kvm_mips_get_cp0_registers(CPUState *cs)
     if (ret < 0) {
         return ret;
     }
+
+    /* If VM clock stopped then state was already saved when it was stopped */
+    if (runstate_is_running()) {
+        ret = kvm_mips_save_count(cs);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
     ret |= kvm_mips_get_one_ulreg(cs, KVM_REG_MIPS_CP0_EPC, &env->CP0_EPC);
     if (ret < 0) {
         return ret;
diff --git a/target-mips/machine.c b/target-mips/machine.c
index 0f36c9e..4916b13 100644
--- a/target-mips/machine.c
+++ b/target-mips/machine.c
@@ -150,6 +150,8 @@ void cpu_save(QEMUFile *f, void *opaque)
         save_tc(f, &env->tcs[i]);
     for (i = 0; i < MIPS_FPU_MAX; i++)
         save_fpu(f, &env->fpus[i]);
+
+    qemu_put_sbe64s(f, &env->count_save_time);
 }
 
 static void load_tc(QEMUFile *f, TCState *tc, int version_id)
@@ -310,5 +312,9 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
 
     /* XXX: ensure compatibility for halted bit ? */
     tlb_flush(CPU(cpu), 1);
+
+    if (version_id >= 5) {
+        qemu_get_sbe64s(f, &env->count_save_time);
+    }
     return 0;
 }
-- 
1.9.3

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

[Paolo Bonzini]

Il 29/05/2014 16:41, James Hogan ha scritto:
> +
> +    /* If VM clock stopped then state was already saved when it was stopped */
> +    if (runstate_is_running()) {
> +        ret = kvm_mips_save_count(cs);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +    }
> +

You're expecting that calls to kvm_mips_get_cp0_registers and 
kvm_mips_put_cp0_registers are balanced and not nested.  Perhaps you 
should add an assert about it.

> +    if (!(count_ctl & KVM_REG_MIPS_COUNT_CTL_DC)) {
> +        count_ctl |= KVM_REG_MIPS_COUNT_CTL_DC;
> +        ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL, &count_ctl);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +    }

Would it make sense to return directly if the master disable bit is set? 
  The rest of this function is idempotent.

Also, perhaps this bit in kvm_mips_restore_count is unnecessary, and so 
is env->count_save_time in general:

> +        /* find time to resume the saved timer at */
> +        now = get_clock();
> +        count_resume = now - (cpu_get_clock_at(now) - env->count_save_time);

Is the COUNT_RESUME write necessary if the VM is running?  Does the 
master disable bit just latch the values, or does it really stop the 
timer?  (My reading of the code is the former, since writing 
COUNT_RESUME only modifies the bias: no write => no bias change => timer 
runs).

And if the VM is not running, you have timer_state.cpu_ticks_enabled == 
false, so cpu_get_clock_at() always returns timers_state.cpu_clock_offset.

So, if the COUNT_RESUME write is not necessary for a running VM, you can 
then just write get_clock() to COUNT_RESUME, which seems to make sense 
to me.

Paolo

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

[James Hogan]

Hi Paolo,

On 29/05/14 16:23, Paolo Bonzini wrote:
> Il 29/05/2014 16:41, James Hogan ha scritto:
>> +
>> +    /* If VM clock stopped then state was already saved when it was
>> stopped */
>> +    if (runstate_is_running()) {
>> +        ret = kvm_mips_save_count(cs);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +    }
>> +
> 
> You're expecting that calls to kvm_mips_get_cp0_registers and
> kvm_mips_put_cp0_registers are balanced and not nested.  Perhaps you
> should add an assert about it.

Yes and no. If you loadvm or do an incoming migration you get an extra
put without a prior get, so it has to handle that case (ensuring that
the timer is frozen in kvm_mips_restore_count). I don't think it should
ever do two kvm_mips_get_cp0_registers calls though, but it should still
handle it okay since the timer will already be frozen so it'd just read
the same values out.

(although as it stands CP0_Count never represents the offset from the VM
clock for KVM like it does with a running Count with TCG, so the vmstate
is technically incompatible between TCG/KVM).

>> +    if (!(count_ctl & KVM_REG_MIPS_COUNT_CTL_DC)) {
>> +        count_ctl |= KVM_REG_MIPS_COUNT_CTL_DC;
>> +        ret = kvm_mips_put_one_reg64(cs, KVM_REG_MIPS_COUNT_CTL,
>> &count_ctl);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +    }
> 
> Would it make sense to return directly if the master disable bit is set?

It would probably indicate that kvm_mips_get_cp0_registers was called
twice, so yes it could do that, although it would probably be
unexpected/wrong if it did happen (maybe qemu modified the values while
the state was dirty).

>  The rest of this function is idempotent.
> 
> Also, perhaps this bit in kvm_mips_restore_count is unnecessary, and so
> is env->count_save_time in general:
> 
>> +        /* find time to resume the saved timer at */
>> +        now = get_clock();
>> +        count_resume = now - (cpu_get_clock_at(now) -
>> env->count_save_time);
> 
> Is the COUNT_RESUME write necessary if the VM is running?

Running at that instant or running continuously since the save?

At this instant the VM is always running. Either it's just been started
and other state isn't dirty, or the registers have been put while the VM
is running.

If the VM wasn't stopped since the context was saved, you're right that
it could skip modifying COUNT_RESUME, it won't have changed. It's there
for if the VM was stopped, e.g.:
stop vm - save
get regs
start vm
put regs - restore (e.g. before run vcpu)

in which case COUNT_RESUME must be altered for Count to keep a similar
offset against the vm clock (like hw/mips/cputimer.c ensures while count
is running - even when vm stopped).

> Does the
> master disable bit just latch the values, or does it really stop the
> timer?  (My reading of the code is the former, since writing
> COUNT_RESUME only modifies the bias: no write => no bias change => timer
> runs).

It appears latched in the sense that starting it again will jump Count
forward to the time it would have been had it not been disabled (with no
loss of Compare interrupt in that time).

However it does stop the timer and interrupts, so if you change Count it
will be as if you changed it exactly at the moment it was latched. If
you change COUNT_RESUME, Count will not change immediately, but the
monotonic time at which the (unchanged) Count value will appear to
resume counting will be changed. So in that sense it acts as a bias.
Increment it by 10ns and the Count will be 10ns behind when you resume
(as if it had been stopped for 10ns, so with no missed interrupts).

> And if the VM is not running, you have timer_state.cpu_ticks_enabled ==
> false, so cpu_get_clock_at() always returns timers_state.cpu_clock_offset.
> 
> So, if the COUNT_RESUME write is not necessary for a running VM, you can
> then just write get_clock() to COUNT_RESUME, which seems to make sense
> to me.

If the VM is stopped you probably wouldn't want to resume the timer yet
at all.

See above though, the VM is always running at this point
(restore_count), even if it has only just started (or may have been
stopped and started since the save).

Does that make sense? It's surprising hard to explain how it works
clearly without resorting to equations :)

Thanks
James

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

[Paolo Bonzini]

Il 29/05/2014 18:27, James Hogan ha scritto:
> (although as it stands CP0_Count never represents the offset from the VM
> clock for KVM like it does with a running Count with TCG, so the vmstate
> is technically incompatible between TCG/KVM).

That can be fixed in cpu_save/cpu_load hooks, like

     if (kvm_enabled()) {
         uint32_t TCGlike_CP0_Count = ...
         qemu_put_sbe32s(f, &TCGlike_CP0_Count);
     } else {
         qemu_put_sbe32s(f, &env->CP0_Count);
     }

...

     if (kvm_enabled()) {
         uint32_t TCGlike_CP0_Count;
         qemu_get_sbe32s(f, &TCGlike_CP0_Count);
         env->CP0_Count = ...
     } else {
         qemu_get_sbe32s(f, &env->CP0_Count);
     }

>> Also, perhaps this bit in kvm_mips_restore_count is unnecessary, and so
>> is env->count_save_time in general:
>>
>>> +        /* find time to resume the saved timer at */
>>> +        now = get_clock();
>>> +        count_resume = now - (cpu_get_clock_at(now) -
>>> env->count_save_time);
>>
>> Is the COUNT_RESUME write necessary if the VM is running?
>
> Running at that instant or running continuously since the save?
>
> At this instant the VM is always running. Either it's just been started
> and other state isn't dirty, or the registers have been put while the VM
> is running.

The possible transitions are:

running, not dirty -> stopped
	need to freeze and load the registers

stopped -> running, not dirty
	will reload the registers, need to modify COUNT_RESUME

running, dirty -> stopped
	no need to do anything

stopped -> running, dirty
	will not reload the registers until put, will need to modify
	COUNT_RESUME on the next transition to "running, not dirty"

running, not dirty -> running, dirty
	need to freeze and load the registers

running, dirty -> running, not dirty
	need to modify COUNT_RESUME if the machine had been stopped
	in the meanwhile

The questions then is, can we skip tracking count_save_time and 
modifying COUNT_RESUME in kvm_mips_restore_count?  Then you can just 
write get_clock() to COUNT_RESUME in kvm_mips_update_state, like this:

     if (!running) {
         if (!cs->kvm_vcpu_dirty) {
             save;
         }
     else {
         write get_clock() to COUNT_RESUME;
         if (!cs->kvm_vcpu_dirty) {
             restore;
         }
     }

and even drop patch 1.  COUNT_RESUME is not even ever read by QEMU nor 
stored in CPUState, so.

The difference is that the guest "loses" the time between the "running, 
not dirty -> running, dirty" and "running, dirty -> stopped" 
transitions, while "gaining" the time between "stopped -> running, 
dirty" and "running, dirty -> running, not dirty".  If this is right, I 
think the difference does not matter in practice and the new/simpler 
code even explains the definition of COUNT_RESUME better in my eyes.

>> Does the
>> master disable bit just latch the values, or does it really stop the
>> timer?  (My reading of the code is the former, since writing
>> COUNT_RESUME only modifies the bias: no write => no bias change => timer
>> runs).
>
> It appears latched in the sense that starting it again will jump Count
> forward to the time it would have been had it not been disabled (with no
> loss of Compare interrupt in that time).

Yes, this is the important part because it means that the guest clock 
does not get progressively more skewed.  It also means that it is right 
to never write COUNT_RESUME except if you go through stop/continue.

Paolo

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

[James Hogan]

Hi Paolo,

On 29/05/14 18:03, Paolo Bonzini wrote:
>>> Also, perhaps this bit in kvm_mips_restore_count is unnecessary, and so
>>> is env->count_save_time in general:
>>>
>>>> +        /* find time to resume the saved timer at */
>>>> +        now = get_clock();
>>>> +        count_resume = now - (cpu_get_clock_at(now) -
>>>> env->count_save_time);
>>>
>>> Is the COUNT_RESUME write necessary if the VM is running?
>>
>> Running at that instant or running continuously since the save?
>>
>> At this instant the VM is always running. Either it's just been started
>> and other state isn't dirty, or the registers have been put while the VM
>> is running.
> 
> The possible transitions are:
> 
> running, not dirty -> stopped
>     need to freeze and load the registers
> 
> stopped -> running, not dirty
>     will reload the registers, need to modify COUNT_RESUME
> 
> running, dirty -> stopped
>     no need to do anything
> 
> stopped -> running, dirty
>     will not reload the registers until put, will need to modify
>     COUNT_RESUME on the next transition to "running, not dirty"
> 
> running, not dirty -> running, dirty
>     need to freeze and load the registers
> 
> running, dirty -> running, not dirty
>     need to modify COUNT_RESUME if the machine had been stopped
>     in the meanwhile
> 
> The questions then is, can we skip tracking count_save_time and
> modifying COUNT_RESUME in kvm_mips_restore_count?  Then you can just
> write get_clock() to COUNT_RESUME in kvm_mips_update_state, like this:
> 
>     if (!running) {
>         if (!cs->kvm_vcpu_dirty) {
>             save;
>         }
>     else {
>         write get_clock() to COUNT_RESUME;
>         if (!cs->kvm_vcpu_dirty) {
>             restore;
>         }
>     }
> 
> and even drop patch 1.  COUNT_RESUME is not even ever read by QEMU nor
> stored in CPUState, so.
> 
> The difference is that the guest "loses" the time between the "running,
> not dirty -> running, dirty" and "running, dirty -> stopped"
> transitions, while "gaining" the time between "stopped -> running,
> dirty" and "running, dirty -> running, not dirty".  If this is right, I
> think the difference does not matter in practice and the new/simpler
> code even explains the definition of COUNT_RESUME better in my eyes.

Yes, I agree with your analysis and had considered something like this,
although it doesn't particularly appeal to my sense of perfectionism :).
It would be race free though, and if you're stopping the VM at all you
expect to lose some time anyway.

> 
>>> Does the
>>> master disable bit just latch the values, or does it really stop the
>>> timer?  (My reading of the code is the former, since writing
>>> COUNT_RESUME only modifies the bias: no write => no bias change => timer
>>> runs).
>>
>> It appears latched in the sense that starting it again will jump Count
>> forward to the time it would have been had it not been disabled (with no
>> loss of Compare interrupt in that time).
> 
> Yes, this is the important part because it means that the guest clock
> does not get progressively more skewed.  It also means that it is right
> to never write COUNT_RESUME except if you go through stop/continue.

Yes, if the VM hasn't been stopped the value written is unchanged from
that originally read (see below), so it could skip it in that case.

save:
  count_save_time = cpu_clock_offset + COUNT_RESUME
restore:
  COUNT_RESUME = get_clock() - (cpu_clock_offset + get_clock() -
count_save_time)
		= count_save_time - cpu_clock_offset
		= COUNT_RESUME

Cheers
James

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

[Paolo Bonzini]

Il 29/05/2014 22:44, James Hogan ha scritto:
> Yes, I agree with your analysis and had considered something like this,
> although it doesn't particularly appeal to my sense of perfectionism :).

I can see that.  But I think the simplification of the code is worth it.

It is hard to explain why the invalid times-goes-backwards case can 
happen if env->count_save_time is overwritten with data from another 
machine.  I think the explanation is that (due to 
timers_state.cpu_ticks_enabled) the value of "cpu_get_clock_at(now) - 
env->count_save_time" does not depend on get_clock(), but the code 
doesn't have any comment for that.

Rather than adding comments, we might as well force it to be always zero 
and just write get_clock() to COUNT_RESUME.

Finally, having to serialize env->count_save_time makes harder to 
support migration from TCG to KVM and back.

> It would be race free though, and if you're stopping the VM at all you
> expect to lose some time anyway.

Since you mentioned perfectionism, :) your code also loses some time; 
COUNT_RESUME is written based on when the CPU state becomes clean, not 
on when the CPU was restarted.

Paolo

Re: [PATCH v2 02/23] MIPS: Export local_flush_icache_range for KVM

[Ralf Baechle]

On Thu, May 29, 2014 at 10:16:24AM +0100, James Hogan wrote:

> Export the local_flush_icache_range function pointer for GPL modules so
> that it can be used by KVM for syncing the icache after binary
> translation of trapping instructions.

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf

Re: [PATCH v2 14/23] MIPS: KVM: Override guest kernel timer frequency directly

[Ralf Baechle]

On Thu, May 29, 2014 at 10:16:36AM +0100, James Hogan wrote:

> 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).

Acked-by: Ralf Baechle <ralf@linux-mips.org>

  Ralf

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

[Paolo Bonzini]

Il 29/05/2014 11:16, James Hogan ha scritto:
> Here are a range of MIPS KVM T&E fixes, preferably for v3.16 but I know
> it's probably a bit late now. Changes are pretty minimal though since
> v1 so please consider. They can also be found on my kvm_mips_queue
> branch (and the kvm_mips_timer_v2 tag) 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-4, 13).
>
> Fixing guest CP0_Count emulation to work without a running host
> CP0_Count (patch 13) 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 10-12) and improvements made thanks to
> valuable feedback from Paolo Bonzini for the last QEMU MIPS/KVM patchset
> (patches 5-7, 15-16).
>
> Finally there are some misc cleanups which I did along the way (patches
> 17-23).
>
> 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-4:
> 	Fix KVM/MIPS with 4K pages, missing RDHWR SYNCI (XBurst),
> 	unmoving CP0_Random (XBurst).
> Patches 5-9:
> 	Add EPC, Count, Compare, UserLocal, HWREna guest CP0 registers
> 	to KVM register ioctl interface.
> Patches 10-12:
> 	Fix a few potential races relating to timers.
> Patches 13-14:
> 	Rewrite guest timer emulation to use ktime_get().
> Patches 15-16:
> 	Add KVM virtual registers for controlling guest timer, including
> 	master timer disable, and timer frequency.
> Patches 17-23:
> 	Cleanups.
>
> Changes in v2 (tag:kvm_mips_timer_v2):
>  Patchset:
>  - Drop patch 4 "MIPS: KVM: Fix CP0_EBASE KVM register id" (David
>    Daney).
>  - Drop patch 14 "MIPS: KVM: Add nanosecond count bias KVM register".
>    The COUNT_CTL and COUNT_RESUME API is clean and sufficient.
>  - Add missing access to UserLocal and HWREna guest CP0 registers
>    (patches 15 and 16).
>  - Add export of local_flush_icache_range (patch 2).
>  Patch 12 MIPS: KVM: Migrate hrtimer to follow VCPU
>  - Move kvm_mips_migrate_count() into kvm_tlb.c to fix a link error when
>    KVM is built as a module, since kvm_tlb.c is built statically and
>    cannot reference symbols in kvm_mips_emul.c.
>  Patch 15 MIPS: KVM: Add master disable count interface
>  - Make KVM_REG_MIPS_COUNT_RESUME writable too so that userland can
>    control timer using master DC and without bias register. New values
>    are rejected if they refer to a monotonic time in the future.
>  - Expand on description of KVM_REG_MIPS_COUNT_RESUME about the effects
>    of the register and that it can be written.
>
> v1 (tag:kvm_mips_timer_v1):
>  see http://marc.info/?l=kvm&m=139843936102657&w=2
>
> James Hogan (23):
>   MIPS: KVM: Allocate at least 16KB for exception handlers
>   MIPS: Export local_flush_icache_range for KVM
>   MIPS: KVM: Use local_flush_icache_range to fix RI on XBurst
>   MIPS: KVM: Use tlb_write_random
>   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: Add CP0_UserLocal KVM register access
>   MIPS: KVM: Add CP0_HWREna 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 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()
>   MIPS: KVM: Remove redundant semicolon
>
>  arch/mips/Kconfig                 |  12 +-
>  arch/mips/include/asm/kvm_host.h  | 183 ++++++++++---
>  arch/mips/include/uapi/asm/kvm.h  |  35 +++
>  arch/mips/kvm/kvm_locore.S        |  32 ---
>  arch/mips/kvm/kvm_mips.c          | 140 +++++-----
>  arch/mips/kvm/kvm_mips_dyntrans.c |  15 +-
>  arch/mips/kvm/kvm_mips_emul.c     | 557 ++++++++++++++++++++++++++++++++++++--
>  arch/mips/kvm/kvm_tlb.c           |  77 +++---
>  arch/mips/kvm/kvm_trap_emul.c     |  86 +++++-
>  arch/mips/mm/cache.c              |   1 +
>  arch/mips/mti-malta/malta-time.c  |  14 +-
>  11 files changed, 920 insertions(+), 232 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: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> Cc: Sanjay Lal <sanjayl@kymasys.com>
>

Applied, thanks.  I hope you'll get the QEMU patches ready in time for 
2.1! :)

Paolo

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

[James Hogan]

On Friday 30 May 2014 13:07:13 Paolo Bonzini wrote:
> Il 29/05/2014 11:16, James Hogan ha scritto:
> > Here are a range of MIPS KVM T&E fixes, preferably for v3.16 but I know
> > it's probably a bit late now. Changes are pretty minimal though since
> > v1 so please consider. They can also be found on my kvm_mips_queue
> > branch (and the kvm_mips_timer_v2 tag) 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-4, 13).
> > 
> > Fixing guest CP0_Count emulation to work without a running host
> > CP0_Count (patch 13) 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 10-12) and improvements made thanks to
> > valuable feedback from Paolo Bonzini for the last QEMU MIPS/KVM patchset
> > (patches 5-7, 15-16).
> > 
> > Finally there are some misc cleanups which I did along the way (patches
> > 17-23).
> > 
> > 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-4:
> > 	Fix KVM/MIPS with 4K pages, missing RDHWR SYNCI (XBurst),
> > 	unmoving CP0_Random (XBurst).
> > 
> > Patches 5-9:
> > 	Add EPC, Count, Compare, UserLocal, HWREna guest CP0 registers
> > 	to KVM register ioctl interface.
> > 
> > Patches 10-12:
> > 	Fix a few potential races relating to timers.
> > 
> > Patches 13-14:
> > 	Rewrite guest timer emulation to use ktime_get().
> > 
> > Patches 15-16:
> > 	Add KVM virtual registers for controlling guest timer, including
> > 	master timer disable, and timer frequency.
> > 
> > Patches 17-23:
> > 	Cleanups.
> > 
> > Changes in v2 (tag:kvm_mips_timer_v2):
> >  Patchset:
> >  - Drop patch 4 "MIPS: KVM: Fix CP0_EBASE KVM register id" (David
> >  
> >    Daney).
> >  
> >  - Drop patch 14 "MIPS: KVM: Add nanosecond count bias KVM register".
> >  
> >    The COUNT_CTL and COUNT_RESUME API is clean and sufficient.
> >  
> >  - Add missing access to UserLocal and HWREna guest CP0 registers
> >  
> >    (patches 15 and 16).
> >  
> >  - Add export of local_flush_icache_range (patch 2).
> >  Patch 12 MIPS: KVM: Migrate hrtimer to follow VCPU
> >  - Move kvm_mips_migrate_count() into kvm_tlb.c to fix a link error when
> >  
> >    KVM is built as a module, since kvm_tlb.c is built statically and
> >    cannot reference symbols in kvm_mips_emul.c.
> >  
> >  Patch 15 MIPS: KVM: Add master disable count interface
> >  - Make KVM_REG_MIPS_COUNT_RESUME writable too so that userland can
> >  
> >    control timer using master DC and without bias register. New values
> >    are rejected if they refer to a monotonic time in the future.
> >  
> >  - Expand on description of KVM_REG_MIPS_COUNT_RESUME about the effects
> >  
> >    of the register and that it can be written.
> > 
> > v1 (tag:kvm_mips_timer_v1):
> >  see http://marc.info/?l=kvm&m=139843936102657&w=2
> > 
> > James Hogan (23):
> >   MIPS: KVM: Allocate at least 16KB for exception handlers
> >   MIPS: Export local_flush_icache_range for KVM
> >   MIPS: KVM: Use local_flush_icache_range to fix RI on XBurst
> >   MIPS: KVM: Use tlb_write_random
> >   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: Add CP0_UserLocal KVM register access
> >   MIPS: KVM: Add CP0_HWREna 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 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()
> >   MIPS: KVM: Remove redundant semicolon
> >  
> >  arch/mips/Kconfig                 |  12 +-
> >  arch/mips/include/asm/kvm_host.h  | 183 ++++++++++---
> >  arch/mips/include/uapi/asm/kvm.h  |  35 +++
> >  arch/mips/kvm/kvm_locore.S        |  32 ---
> >  arch/mips/kvm/kvm_mips.c          | 140 +++++-----
> >  arch/mips/kvm/kvm_mips_dyntrans.c |  15 +-
> >  arch/mips/kvm/kvm_mips_emul.c     | 557
> >  ++++++++++++++++++++++++++++++++++++-- arch/mips/kvm/kvm_tlb.c          
> >  |  77 +++---
> >  arch/mips/kvm/kvm_trap_emul.c     |  86 +++++-
> >  arch/mips/mm/cache.c              |   1 +
> >  arch/mips/mti-malta/malta-time.c  |  14 +-
> >  11 files changed, 920 insertions(+), 232 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: Andreas Herrmann <andreas.herrmann@caviumnetworks.com>
> > Cc: Sanjay Lal <sanjayl@kymasys.com>
> 
> Applied, thanks.

Thanks Paolo & Ralf!

> I hope you'll get the QEMU patches ready in time for
> 2.1! :)

Me too, 2.1 is the plan.

Cheers
James

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

[James Hogan]

On 30/05/14 08:57, Paolo Bonzini wrote:
> Il 29/05/2014 22:44, James Hogan ha scritto:
>> Yes, I agree with your analysis and had considered something like this,
>> although it doesn't particularly appeal to my sense of perfectionism :).
> 
> I can see that.  But I think the simplification of the code is worth it.
> 
> It is hard to explain why the invalid times-goes-backwards case can
> happen if env->count_save_time is overwritten with data from another
> machine.  I think the explanation is that (due to
> timers_state.cpu_ticks_enabled) the value of "cpu_get_clock_at(now) -
> env->count_save_time" does not depend on get_clock(), but the code
> doesn't have any comment for that.

Exactly. I think of it in terms of count_save_time being in the time
frame of the vm clock, which is also migrated.

In fact since the VM clock is stopped during migration, the
saved/restored count_save_time will likely be the same as the saved vm
clock, so the delta calculated above at restore time is the time since
the vm clock was resumed.

> Rather than adding comments, we might as well force it to be always zero
> and just write get_clock() to COUNT_RESUME.
> 
> Finally, having to serialize env->count_save_time makes harder to
> support migration from TCG to KVM and back.

Yes, I'm not keen on that bit of code.

> 
>> It would be race free though, and if you're stopping the VM at all you
>> expect to lose some time anyway.
> 
> Since you mentioned perfectionism, :) your code also loses some time;
> COUNT_RESUME is written based on when the CPU state becomes clean, not
> on when the CPU was restarted.

The offset you suggest removing is what ensures time isn't lost there.
The VM time when the cpu is restarted == the VM time when it is stopped,
so by saving vm time to count_save_time at vm stop, the timer can be
restarted at exactly the right interval into the past
(COUNT_RESUME=now-interval) so that no time is lost.

Now there is one case I believe where time will be gained which is hard
to avoid without getting a notification before VM stop rather than
after. When the VM is stopped and qemu's state is clean, the state is
read very soon after (not immediately), so the saved state will be at
slightly after the recorded vm time. I.e. the timer was allowed to
continue slightly past when the vm clock was actually stopped.

Cheers
James

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

[Paolo Bonzini]

Il 16/06/2014 18:29, James Hogan ha scritto:
>> Rather than adding comments, we might as well force it to be always zero
>> and just write get_clock() to COUNT_RESUME.
>>
>> Finally, having to serialize env->count_save_time makes harder to
>> support migration from TCG to KVM and back.
>
> Yes, I'm not keen on that bit of code.

Are you going to submit it for 2.1?  There are still a few days for 
review and inclusion.

Paolo