Re: vmm(4): use monotonic base for pvclock

2021-06-01 Thread Mike Larkin
On Tue, Jun 01, 2021 at 08:03:43PM -0500, Scott Cheloha wrote:
> The documentation for the Linux pvclock is pretty sparse but I am
> pretty sure we want to use a monotonic base for ti_system_time.  We
> also have a function for converting a timespec into a 64-bit count of
> nanoseconds we can use.
>
> We may as well also use rdtsc_lfence() to ensure consistent behavior.
>
> ... this is still not quite right because the VM expects the pvclock
> to have a fixed frequency, but we have no interface to reading a raw
> timestamp.  Something to add in the future, maybe.
>
> Index: vmm.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.284
> diff -u -p -r1.284 vmm.c
> --- vmm.c 18 May 2021 00:05:20 -  1.284
> +++ vmm.c 2 Jun 2021 00:57:31 -
> @@ -7294,8 +7294,8 @@ vmm_init_pvclock(struct vcpu *vcpu, padd
>  int
>  vmm_update_pvclock(struct vcpu *vcpu)
>  {
> + struct timespec now;
>   struct pvclock_time_info *pvclock_ti;
> - struct timespec tv;
>   struct vm *vm = vcpu->vc_parent;
>   paddr_t pvclock_hpa, pvclock_gpa;
>
> @@ -7309,10 +7309,9 @@ vmm_update_pvclock(struct vcpu *vcpu)
>   pvclock_ti->ti_version =
>   (++vcpu->vc_pvclock_version << 1) | 0x1;
>
> - pvclock_ti->ti_tsc_timestamp = rdtsc();
> - nanotime(&tv);
> - pvclock_ti->ti_system_time =
> - tv.tv_sec * 10L + tv.tv_nsec;
> + pvclock_ti->ti_tsc_timestamp = rdtsc_lfence();
> + nanouptime(&now);
> + pvclock_ti->ti_system_time = TIMESPEC_TO_NSEC(&now);
>   pvclock_ti->ti_tsc_shift = 12;
>   pvclock_ti->ti_tsc_to_system_mul =
>   vcpu->vc_pvclock_system_tsc_mul;
>

This probably needs to be tested on a wide variety (and versions) of Linux
guests. I've found in the past that different kernel versions do different
things and behave differently.

Did you test a few Linux guest VMs? Did this work across all of them?

-ml



vmm(4): use monotonic base for pvclock

2021-06-01 Thread Scott Cheloha
The documentation for the Linux pvclock is pretty sparse but I am
pretty sure we want to use a monotonic base for ti_system_time.  We
also have a function for converting a timespec into a 64-bit count of
nanoseconds we can use.

We may as well also use rdtsc_lfence() to ensure consistent behavior.

... this is still not quite right because the VM expects the pvclock
to have a fixed frequency, but we have no interface to reading a raw
timestamp.  Something to add in the future, maybe.

Index: vmm.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.284
diff -u -p -r1.284 vmm.c
--- vmm.c   18 May 2021 00:05:20 -  1.284
+++ vmm.c   2 Jun 2021 00:57:31 -
@@ -7294,8 +7294,8 @@ vmm_init_pvclock(struct vcpu *vcpu, padd
 int
 vmm_update_pvclock(struct vcpu *vcpu)
 {
+   struct timespec now;
struct pvclock_time_info *pvclock_ti;
-   struct timespec tv;
struct vm *vm = vcpu->vc_parent;
paddr_t pvclock_hpa, pvclock_gpa;
 
@@ -7309,10 +7309,9 @@ vmm_update_pvclock(struct vcpu *vcpu)
pvclock_ti->ti_version =
(++vcpu->vc_pvclock_version << 1) | 0x1;
 
-   pvclock_ti->ti_tsc_timestamp = rdtsc();
-   nanotime(&tv);
-   pvclock_ti->ti_system_time =
-   tv.tv_sec * 10L + tv.tv_nsec;
+   pvclock_ti->ti_tsc_timestamp = rdtsc_lfence();
+   nanouptime(&now);
+   pvclock_ti->ti_system_time = TIMESPEC_TO_NSEC(&now);
pvclock_ti->ti_tsc_shift = 12;
pvclock_ti->ti_tsc_to_system_mul =
vcpu->vc_pvclock_system_tsc_mul;