Tracked down some issues this week with linux guests complaining about
calibrating kvm-clock and found we have a few isues:

1. we now need to centralize the pvclock epoch on the vm as vmd is now
   smp-capable
2. suspend/resume should tell guests that watchdogs should not freak
   out. Linux uses a bit to communicate this and the guest clears it.
3. we had some logic issues in the memory range lookups. (the function
   isn't used by other parts of vmm.)
4. we should use tsc offsetting support in VMX and SVM to properly cross
   a hibernate boundary where the host cpus are powered off

I've tested this on my Intel and AMD laptop I have with me this week,
but this would benefit from tests from folks running a variety of
versions of Linux guests.

-dv


diff /usr/src
path + /usr/src
commit - 5d5aa7a8a8ab23fdd6c67a2d4c3c4ef12c895af6
blob - 8644a0cc6239affd8d0b9891197e44b6f46968d0
file + sys/arch/amd64/amd64/vmm_machdep.c
--- sys/arch/amd64/amd64/vmm_machdep.c
+++ sys/arch/amd64/amd64/vmm_machdep.c
@@ -70,7 +70,10 @@ void vmx_dump_vmcs_field(uint16_t, const char *);
 int vmm_enabled(void);
 void vmm_activate_machdep(struct device *, int);
 int vmmioctl_machdep(dev_t, u_long, caddr_t, int, struct proc *);
+int vmm_quiesce_svm(void);
 int vmm_quiesce_vmx(void);
+void vmm_pvclock_suspend(void);
+void vmm_pvclock_resume(void);
 int vm_run(struct vm_run_params *);
 int vm_intr_pending(struct vm_intr_params *);
 int vm_rwregs(struct vm_rwregs_params *, int);
@@ -339,6 +342,43 @@ vmm_attach_machdep(struct device *parent, struct devic
 	rw_init(&sc->vpid_lock, "vpid");
 }

+int
+vmm_quiesce_svm(void)
+{
+	struct vm		*vm;
+	struct vcpu		*vcpu;
+	int			 err;
+
+	/*
+	 * We should be only called from a quiescing device state so we
+	 * don't expect to sleep here. If we can't get all our locks,
+	 * something is wrong.
+	 */
+	if ((err = rw_enter(&vmm_softc->vm_lock, RW_WRITE | RW_NOSLEEP)))
+		return (err);
+
+	/* Iterate over each vm... */
+	SLIST_FOREACH(vm, &vmm_softc->vm_list, vm_link) {
+		/* Iterate over each vcpu... */
+		SLIST_FOREACH(vcpu, &vm->vm_vcpu_list, vc_vcpu_link) {
+			err = rw_enter(&vcpu->vc_lock, RW_WRITE | RW_NOSLEEP);
+			if (err)
+				break;
+
+			/* Flag vcpu as having been stopped. */
+			vcpu->vc_pvclock_stopped = 1;
+			rw_exit_write(&vcpu->vc_lock);
+		}
+		if (err)
+			break;
+	}
+	rw_exit_write(&vmm_softc->vm_lock);
+
+	if (err)
+		return (err);
+	return (0);
+}
+
 /*
  * vmm_quiesce_vmx
  *
@@ -367,6 +407,9 @@ vmm_quiesce_vmx(void)
 			if (err)
 				break;

+			/* Flag vcpu as having been stopped. */
+			vcpu->vc_pvclock_stopped = 1;
+
 			/* We can skip unlaunched VMCS. Nothing to flush. */
 			if (atomic_load_int(&vcpu->vc_vmx_vmcs_state)
 			    != VMCS_LAUNCHED) {
@@ -425,15 +468,24 @@ vmm_activate_machdep(struct device *self, int act)
 			break;

 		/* Intel systems need extra steps to sync vcpu state. */
-		if (vmm_softc->mode == VMM_MODE_EPT)
+		if (vmm_softc->mode == VMM_MODE_EPT) {
 			if (vmm_quiesce_vmx())
 				DPRINTF("%s: vmx quiesce failed\n", __func__);
+		} else if (vmm_softc->mode == VMM_MODE_RVI) {
+			if (vmm_quiesce_svm())
+				DPRINTF("%s: svm quiesce failed\n", __func__);
+		}

+		vmm_pvclock_suspend();
+
 		/* Stop virtualization mode on all cpus. */
 		vmm_stop();
+
 		break;

 	case DVACT_WAKEUP:
+		vmm_pvclock_resume();
+
 		/* Restart virtualization mode on all cpu's. */
 		if (vmm_softc->vm_ct > 0)
 			vmm_start();
@@ -441,6 +493,67 @@ vmm_activate_machdep(struct device *self, int act)
 	}
 }

+/* All device users have been drained by vmm_activate(). */
+void
+vmm_pvclock_suspend(void)
+{
+	struct vm *vm;
+	uint64_t tsc, uptime;
+	u_long s;
+
+	if (tsc_frequency == 0)
+		return;
+
+	KASSERT(vmm_softc->sc_status == VMM_SUSPENDED);
+	rw_enter_write(&vmm_softc->vm_lock);
+	s = intr_disable();
+	tsc = rdtsc_lfence();
+	uptime = nsecuptime();
+	intr_restore(s);
+	SLIST_FOREACH(vm, &vmm_softc->vm_list, vm_link) {
+		vm->vm_pvclock_suspend_tsc = tsc + vm->vm_pvclock_tsc_offset;
+		vm->vm_pvclock_suspend_time = uptime;
+	}
+	rw_exit_write(&vmm_softc->vm_lock);
+}
+
+void
+vmm_pvclock_resume(void)
+{
+	struct vm *vm;
+	uint64_t tsc, uptime, elapsed, nsec;
+	u_long s;
+
+	if (tsc_frequency == 0)
+		return;
+
+	KASSERT(vmm_softc->sc_status == VMM_SUSPENDED);
+	rw_enter_write(&vmm_softc->vm_lock);
+	s = intr_disable();
+	tsc = rdtsc_lfence();
+	uptime = nsecuptime();
+	intr_restore(s);
+	SLIST_FOREACH(vm, &vmm_softc->vm_list, vm_link) {
+		if (vm->vm_pvclock_suspend_time == 0)
+			continue;
+
+		/*
+		 * Uptime includes the time asleep, but the host TSC may have
+		 * stopped or reset. Advance the saved guest TSC by that time.
+		 * Split the conversion to avoid overflowing the products.
+		 */
+		elapsed = uptime - vm->vm_pvclock_suspend_time;
+		nsec = elapsed % 1000000000ULL;
+		elapsed = (elapsed / 1000000000ULL) * tsc_frequency +
+		    nsec * (tsc_frequency / 1000000000ULL) +
+		    nsec * (tsc_frequency % 1000000000ULL) / 1000000000ULL;
+		vm->vm_pvclock_tsc_offset =
+		    vm->vm_pvclock_suspend_tsc + elapsed - tsc;
+		vm->vm_pvclock_suspend_time = 0;
+	}
+	rw_exit_write(&vmm_softc->vm_lock);
+}
+
 int
 vmmioctl_machdep(dev_t dev, u_long cmd, caddr_t data, int flag, struct proc *p)
 {
@@ -565,17 +678,25 @@ vm_rwvmparams(struct vm_rwvmparams_params *vpp, int di
 	}

 	if (dir == 0) {
+		rw_enter_read(&vcpu->vc_lock);
 		if (vpp->vpp_mask & VM_RWVMPARAMS_PVCLOCK_VERSION)
 			vpp->vpp_pvclock_version = vcpu->vc_pvclock_version;
 		if (vpp->vpp_mask & VM_RWVMPARAMS_PVCLOCK_SYSTEM_GPA)
 			vpp->vpp_pvclock_system_gpa = \
 			    vcpu->vc_pvclock_system_gpa;
+		if (vpp->vpp_mask & VM_RWVMPARAMS_PVCLOCK_STOPPED)
+			vpp->vpp_pvclock_stopped = vcpu->vc_pvclock_stopped;
+		rw_exit_read(&vcpu->vc_lock);
 	} else {
+		rw_enter_write(&vcpu->vc_lock);
 		if (vpp->vpp_mask & VM_RWVMPARAMS_PVCLOCK_VERSION)
 			vcpu->vc_pvclock_version = vpp->vpp_pvclock_version;
-		if (vpp->vpp_mask & VM_RWVMPARAMS_PVCLOCK_SYSTEM_GPA) {
+		if (vpp->vpp_mask & VM_RWVMPARAMS_PVCLOCK_SYSTEM_GPA)
 			vmm_init_pvclock(vcpu, vpp->vpp_pvclock_system_gpa);
-		}
+		if (vpp->vpp_mask & VM_RWVMPARAMS_PVCLOCK_STOPPED)
+			vcpu->vc_pvclock_stopped =
+			    (vpp->vpp_pvclock_stopped ? 1 : 0);
+		rw_exit_write(&vcpu->vc_lock);
 	}
 out:
 	refcnt_rele_wake(&vm->vm_refcnt);
@@ -928,6 +1049,9 @@ vmx_remote_vmclear(struct cpu_info *ci, struct vcpu *v
 int
 vm_impl_init(struct vm *vm, struct proc *p)
 {
+	struct timespec tv;
+	u_long s;
+
 	/* If not EPT or RVI, nothing to do here */
 	switch (vmm_softc->mode) {
 	case VMM_MODE_EPT:
@@ -941,6 +1065,18 @@ vm_impl_init(struct vm *vm, struct proc *p)
 		return (EINVAL);
 	}

+	/* Establish one pvclock epoch before vcpus run and perform updates. */
+	if (tsc_frequency > 0) {
+		vm->vm_pvclock_tsc_mul =
+		    (1000000000ULL << 20) / tsc_frequency;
+		s = intr_disable();
+		vm->vm_pvclock_tsc = rdtsc_lfence();
+		nanouptime(&tv);
+		intr_restore(s);
+		vm->vm_pvclock_time =
+		    tv.tv_sec * 1000000000ULL + tv.tv_nsec;
+	}
+
 	return (0);
 }

@@ -993,6 +1129,9 @@ vcpu_reload_vmcs_vmx(struct vcpu *vcpu)
 		return (EINVAL);
 	}

+	if (vmwrite(VMCS_TSC_OFFSET, vcpu->vc_parent->vm_pvclock_tsc_offset))
+		return (EINVAL);
+
 	return (0);
 }

@@ -2117,6 +2256,7 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg
 	 * Procbased ctrls
 	 *
 	 * We must be able to set the following:
+	 * IA32_VMX_USE_TSC_OFFSETTING - preserve guest TSC across suspend
 	 * IA32_VMX_HLT_EXITING - exit on HLT instruction
 	 * IA32_VMX_MWAIT_EXITING - exit on MWAIT instruction
 	 * IA32_VMX_UNCONDITIONAL_IO_EXITING - exit on I/O instructions
@@ -2130,7 +2270,8 @@ vcpu_reset_regs_vmx(struct vcpu *vcpu, struct vcpu_reg
 	 * IA32_VMX_CR3_LOAD_EXITING - don't care about guest CR3 accesses
 	 * IA32_VMX_CR3_STORE_EXITING - don't care about guest CR3 accesses
 	 */
-	want1 = IA32_VMX_HLT_EXITING |
+	want1 = IA32_VMX_USE_TSC_OFFSETTING |
+	    IA32_VMX_HLT_EXITING |
 	    IA32_VMX_MWAIT_EXITING |
 	    IA32_VMX_UNCONDITIONAL_IO_EXITING |
 	    IA32_VMX_USE_MSR_BITMAPS |
@@ -6762,6 +6903,11 @@ vcpu_run_svm(struct vcpu *vcpu, struct vm_run_params *
 	uint64_t exit_reason;
 	struct vmcb *vmcb = (struct vmcb *)vcpu->vc_control_va;

+	if (vmcb->v_tsc_offset != vcpu->vc_parent->vm_pvclock_tsc_offset) {
+		vmcb->v_tsc_offset = vcpu->vc_parent->vm_pvclock_tsc_offset;
+		svm_set_dirty(vcpu, SVM_CLEANBITS_I);
+	}
+
 	/* See vcpu_run_vmx(): preserve assertions racing VMM_IOC_RUN entry. */
 	vcpu->vc_intr = vrp->vrp_intr_pending |
 	    atomic_swap_uint(&vcpu->vc_intr_latch, 0);
@@ -7118,15 +7264,16 @@ vmm_free_vpid(uint16_t vpid)

 /* vmm_gpa_is_valid
  *
- * Check if the given gpa is within guest memory space.
+ * Check if the given gpa and oject size are within a single guest
+ * memory range not associated with MMIO.
  *
  * Parameters:
  *	vcpu: The virtual cpu we are running on.
- *	gpa: The address to check.
+ *	gpa: The base guest physical address.
  *	obj_size: The size of the object assigned to gpa
  *
  * Return values:
- *	1: gpa is within the memory ranges allocated for the vcpu
+ *	1: gpa range is within an allowed guest memory range
  *	0: otherwise
  */
 int
@@ -7138,9 +7285,10 @@ vmm_gpa_is_valid(struct vcpu *vcpu, paddr_t gpa, size_

 	for (i = 0; i < vm->vm_nmemranges; ++i) {
 		vmr = &vm->vm_memranges[i];
-		if (vmr->vmr_size >= obj_size &&
-		    vmr->vmr_gpa <= gpa &&
-		    gpa < (vmr->vmr_gpa + vmr->vmr_size - obj_size)) {
+		if (vmr->vmr_type != VM_MEM_MMIO &&
+		    vmr->vmr_size >= obj_size &&
+		    gpa >= vmr->vmr_gpa &&
+		    gpa - vmr->vmr_gpa <= vmr->vmr_size - obj_size) {
 		    return 1;
 		}
 	}
@@ -7166,11 +7314,6 @@ vmm_init_pvclock(struct vcpu *vcpu, paddr_t gpa)
 	}

 	vcpu->vc_pvclock_system_gpa = gpa;
-	if (tsc_frequency > 0)
-		vcpu->vc_pvclock_system_tsc_mul =
-		    (int) ((1000000000L << 20) / tsc_frequency);
-	else
-		vcpu->vc_pvclock_system_tsc_mul = 0;

 	if (vmm_update_pvclock(vcpu) != 0)
 		return (VMM_ACTION_TERMINATE);
@@ -7182,9 +7325,9 @@ int
 vmm_update_pvclock(struct vcpu *vcpu)
 {
 	struct pvclock_time_info *pvclock_ti;
-	struct timespec tv;
 	struct vm *vm = vcpu->vc_parent;
 	paddr_t pvclock_hpa, pvclock_gpa;
+	uint64_t delta;

 	if (vcpu->vc_pvclock_system_gpa & PVCLOCK_SYSTEM_TIME_ENABLE) {
 		pvclock_gpa = vcpu->vc_pvclock_system_gpa & 0xFFFFFFFFFFFFFFF0;
@@ -7192,21 +7335,36 @@ vmm_update_pvclock(struct vcpu *vcpu)
 			return (EINVAL);
 		pvclock_ti = (void*) PMAP_DIRECT_MAP(pvclock_hpa);

+		/*
+		 * Advance the anchor using the guest TSC in units of 2^20
+		 * cycles. Leaving the fractional conversion to the
+		 * guest preserves the same rounding across vcpus and
+		 * avoids overflowing the product.
+		 */
+		delta = (rdtsc_lfence() + vm->vm_pvclock_tsc_offset -
+		    vm->vm_pvclock_tsc) >> 20;
+
 		/* START next cycle (must be odd) */
 		pvclock_ti->ti_version =
-		    (++vcpu->vc_pvclock_version << 1) | 0x1;
+		    (vcpu->vc_pvclock_version << 1) | 0x1;
+		membar_producer();

-		pvclock_ti->ti_tsc_timestamp = rdtsc();
-		nanouptime(&tv);
-		pvclock_ti->ti_system_time =
-		    tv.tv_sec * 1000000000L + tv.tv_nsec;
+		pvclock_ti->ti_tsc_timestamp =
+		    vm->vm_pvclock_tsc + (delta << 20);
+		pvclock_ti->ti_system_time = vm->vm_pvclock_time +
+		    delta * vm->vm_pvclock_tsc_mul;
 		pvclock_ti->ti_tsc_shift = 12;
-		pvclock_ti->ti_tsc_to_system_mul =
-		    vcpu->vc_pvclock_system_tsc_mul;
-		pvclock_ti->ti_flags = PVCLOCK_FLAG_TSC_STABLE;
+		pvclock_ti->ti_tsc_to_system_mul = vm->vm_pvclock_tsc_mul;
+		pvclock_ti->ti_flags = PVCLOCK_FLAG_TSC_STABLE |
+		    (pvclock_ti->ti_flags & PVCLOCK_FLAG_GUEST_STOPPED);
+		if (vcpu->vc_pvclock_stopped)
+			pvclock_ti->ti_flags |= PVCLOCK_FLAG_GUEST_STOPPED;

 		/* END (must be even) */
-		pvclock_ti->ti_version &= ~0x1;
+		membar_producer();
+		pvclock_ti->ti_version = ++vcpu->vc_pvclock_version << 1;
+
+		vcpu->vc_pvclock_stopped = 0;
 	}
 	return (0);
 }
commit - 5d5aa7a8a8ab23fdd6c67a2d4c3c4ef12c895af6
blob - 091201594c2a6566f13bf4a479ec061e4819b8be
file + sys/arch/amd64/include/vmmvar.h
--- sys/arch/amd64/include/vmmvar.h
+++ sys/arch/amd64/include/vmmvar.h
@@ -989,7 +989,7 @@ struct vcpu {

 	uint32_t vc_pvclock_version;		/* [v] */
 	paddr_t vc_pvclock_system_gpa;		/* [v] */
-	uint32_t vc_pvclock_system_tsc_mul;	/* [v] */
+	uint8_t vc_pvclock_stopped;		/* [v] */

 	/* Shadowed MSRs */
 	uint64_t vc_shadow_pat;			/* [v] */
commit - 5d5aa7a8a8ab23fdd6c67a2d4c3c4ef12c895af6
blob - 7c0736cba39f7bcc1b16b62338dfe46700a9def4
file + sys/dev/pv/pvreg.h
--- sys/dev/pv/pvreg.h
+++ sys/dev/pv/pvreg.h
@@ -65,6 +65,7 @@ struct pvclock_time_info {
 } __packed;

 #define PVCLOCK_FLAG_TSC_STABLE			0x01
+#define PVCLOCK_FLAG_GUEST_STOPPED		0x02
 #define PVCLOCK_SYSTEM_TIME_ENABLE		0x01

 /*
commit - 5d5aa7a8a8ab23fdd6c67a2d4c3c4ef12c895af6
blob - f7b7aea324db743a6afdadfa771f2da059acb3a2
file + sys/dev/vmm/vmm.h
--- sys/dev/vmm/vmm.h
+++ sys/dev/vmm/vmm.h
@@ -133,8 +133,9 @@ struct vm_run_params {

 #define VM_RWVMPARAMS_PVCLOCK_SYSTEM_GPA 0x1	/* read/write pvclock gpa */
 #define VM_RWVMPARAMS_PVCLOCK_VERSION	 0x2	/* read/write pvclock version */
+#define VM_RWVMPARAMS_PVCLOCK_STOPPED	 0x4	/* read/write pvclock stopped */
 #define VM_RWVMPARAMS_ALL	(VM_RWVMPARAMS_PVCLOCK_SYSTEM_GPA | \
-    VM_RWVMPARAMS_PVCLOCK_VERSION)
+    VM_RWVMPARAMS_PVCLOCK_VERSION | VM_RWVMPARAMS_PVCLOCK_STOPPED)

 struct vm_rwvmparams_params {
 	/* Input parameters to VMM_IOC_READVMPARAMS/VMM_IOC_WRITEVMPARAMS */
@@ -143,6 +144,7 @@ struct vm_rwvmparams_params {
 	uint32_t		vpp_mask;
 	paddr_t			vpp_pvclock_system_gpa;
 	uint32_t		vpp_pvclock_version;
+	uint8_t			vpp_pvclock_stopped;
 };

 /* IOCTL definitions */
@@ -184,6 +186,7 @@ enum {
  *	I	immutable after create
  *	K	kernel lock
  *	r	reference count
+ *	S	written only while vmm is suspended and sc_refcnt is drained
  *	v	vcpu list rwlock (vm_vcpu_list)
  *	V	vmm_softc's vm_lock
  */
@@ -206,6 +209,13 @@ struct vm {
 	uint32_t		 vm_vcpu_ct;		/* [v] */
 	struct rwlock		 vm_vcpu_lock;

+	uint64_t		 vm_pvclock_tsc;		/* [I] */
+	uint64_t		 vm_pvclock_tsc_offset;	/* [S] */
+	uint64_t		 vm_pvclock_time;	/* [I] */
+	uint32_t		 vm_pvclock_tsc_mul;	/* [I] */
+	uint64_t		 vm_pvclock_suspend_tsc;	/* [S] */
+	uint64_t		 vm_pvclock_suspend_time; /* [S] */
+
 	SLIST_ENTRY(vm)		 vm_link;		/* [V] */
 };