Re: [win-pv-devel] [PATCH] Wallclock Time Calculation Checks Update Versions For Consistency

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Mackay, Eric [mailto:mackayem@xxxxxxxxxx]
> Sent: 21 August 2017 19:44
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; win-pv-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: RE: [win-pv-devel] [PATCH] Wallclock Time Calculation Checks
> Update Versions For Consistency
> 
> I tested the change on XenServer 7.1 on Windows Server 2008 R2, Windows
> Server 2012, and Windows Server 2012 R2. You can run this powershell
> command to see the difference between the dom0 time and the domU
> time: ("OS: {0:o}`r`nXenTime: {1:o}" -f (Get-Date).ToUniversalTime(),
> [DateTime]::FromFileTimeUtc((Get-WmiObject -Namespace root\wmi -Class
> XenProjectXenStoreBase).XenTime))
> 
> It's off by at most a few seconds, which I have read is about the limit of 
> cross-
> domain timekeeping accuracy in Xen.

Thanks Eric. That sounds fine. I'll try to get this patch into Citrix's 
automated testing as soon as I can.

Cheers,

  Paul

> 
> -----Original Message-----
> From: Paul Durrant [mailto:Paul.Durrant@xxxxxxxxxx]
> Sent: Monday, August 21, 2017 7:58 AM
> To: Mackay, Eric <mackayem@xxxxxxxxxx>; win-pv-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: RE: [win-pv-devel] [PATCH] Wallclock Time Calculation Checks
> Update Versions For Consistency
> 
> > -----Original Message-----
> > From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx]
> > On Behalf Of Eric Mackay
> > Sent: 19 August 2017 01:23
> > To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: Eric Mackay <mackayem@xxxxxxxxxx>
> > Subject: [win-pv-devel] [PATCH] Wallclock Time Calculation Checks
> > Update Versions For Consistency
> >
> > Checking the shared_info update versions is necessary to get a
> > consistent set of values. The version is incremented once when the
> > update starts, and then incremented again after the update has
> > completed. To verify that a set of values obtained from shared_info is
> > consistent, the version must not only look at equality of versions,
> > but the version must also be even. Data can only be safely be captured
> > within the version check loop.
> >
> > There is no need to use a hypercall to get the system time, since this
> > is alredy captured in the shared_info struct. A cached version of the
> > time since boot is stored in structures for each vcpu, but this has to
> > be combined with the timestamp counter and some scaling factors to get
> > the actual current time since boot.
> >
> > Clock synchronization can also occur, and the dom0 will ensure that
> > the values in the shared_info and vcpu_time_info structs are kept
> > current to reflect this.
> >
> > Signed-off-by: Eric Mackay <mackayem@xxxxxxxxxx>
> 
> Eric,
> 
> The change looks good to me. What sort of testing has been done on this?
> 
>   Cheers,
> 
>     Paul
> 
> > ---
> >  src/xenbus/shared_info.c | 65
> > ++++++++++++++++++++++++++++++++++++------------
> >  1 file changed, 49 insertions(+), 16 deletions(-)
> >
> > diff --git a/src/xenbus/shared_info.c b/src/xenbus/shared_info.c index
> > d6babcf..2666f76 100644
> > --- a/src/xenbus/shared_info.c
> > +++ b/src/xenbus/shared_info.c
> > @@ -329,6 +329,10 @@ SharedInfoEvtchnUnmask(
> >      return SharedInfoTestBit(&Shared->evtchn_pending[SelectorBit],
> > PortBit);
> >  }
> >
> > +#ifndef BooleanFlagOn
> > +#define BooleanFlagOn(F,SF)   ((BOOLEAN)(((F) & (SF)) != 0))
> > +#endif
> > +
> >  static LARGE_INTEGER
> >  SharedInfoGetTime(
> >      IN  PINTERFACE              Interface
> > @@ -336,51 +340,80 @@ SharedInfoGetTime(  {
> >      PXENBUS_SHARED_INFO_CONTEXT Context = Interface->Context;
> >      shared_info_t               *Shared;
> > -    ULONG                       Version;
> > +    ULONG                       WcVersion;
> > +    ULONG                       TimeVersion;
> >      ULONGLONG                   Seconds;
> >      ULONGLONG                   NanoSeconds;
> > +    ULONGLONG                   Timestamp;
> > +    ULONGLONG                   Tsc;
> > +    ULONGLONG                   SystemTime;
> > +    ULONG                       TscSystemMul;
> > +    CHAR                        TscShift;
> >      LARGE_INTEGER               Now;
> >      TIME_FIELDS                 Time;
> >      KIRQL                       Irql;
> > -    NTSTATUS                    status;
> >
> >      // Make sure we don't suspend
> > -    KeRaiseIrql(DISPATCH_LEVEL, &Irql);
> > +    KeRaiseIrql(DISPATCH_LEVEL, &Irql);
> >
> >      Shared = Context->Shared;
> >
> > +    // Loop until we can read a consistent set of values from the
> > + same update
> >      do {
> > -        Version = Shared->wc_version;
> > +        WcVersion = Shared->wc_version;
> > +        TimeVersion = Shared->vcpu_info[0].time.version;
> >          KeMemoryBarrier();
> >
> > +        // Wallclock time at system time zero (guest boot or resume)
> >          Seconds = Shared->wc_sec;
> >          NanoSeconds = Shared->wc_nsec;
> > +
> > +        // Cached time in nanoseconds since guest boot
> > +        SystemTime = Shared->vcpu_info[0].time.system_time;
> > +
> > +        // Timestamp counter value when these time values were last
> updated
> > +        Timestamp = Shared->vcpu_info[0].time.tsc_timestamp;
> > +
> > +        // Timestamp modifiers
> > +        TscShift = Shared->vcpu_info[0].time.tsc_shift;
> > +        TscSystemMul = Shared->vcpu_info[0].time.tsc_to_system_mul;
> >          KeMemoryBarrier();
> > -    } while (Shared->wc_version != Version);
> >
> > -    // Get the number of nanoseconds since boot
> > -    status = HvmGetTime(&Now);
> > -    if (!NT_SUCCESS(status))
> > -        Now.QuadPart = Shared->vcpu_info[0].time.system_time;
> > +    // Version is incremented to indicate update in progress
> > +    // LSB of version is set if update in progress
> > +    // Version is incremented again once update has completed
> > +    } while (Shared->wc_version != WcVersion ||
> > +             Shared->vcpu_info[0].time.version != TimeVersion ||
> > +             BooleanFlagOn(WcVersion, 0x1) ||
> > +             BooleanFlagOn(TimeVersion, 0x1));
> > +
> > +    // Read counter ticks
> > +    Tsc = ReadTimeStampCounter();
> >
> >      KeLowerIrql(Irql);
> >
> > -    Trace("WALLCLOCK: Seconds = %llu NanoSeconds = %llu\n",
> > +    // Number of elapsed ticks since timestamp was captured
> > +    Tsc -= Timestamp;
> > +
> > +    // Time in nanoseconds since boot
> > +    SystemTime += ((Tsc << TscShift) * TscSystemMul) >> 32;
> > +
> > +    Trace("WALLCLOCK TIME AT BOOT: Seconds = %llu NanoSeconds =
> > %llu\n",
> >            Seconds,
> >            NanoSeconds);
> >
> > -    Trace("BOOT: Seconds = %llu NanoSeconds = %llu\n",
> > -          Now.QuadPart / 1000000000ull,
> > -          Now.QuadPart % 1000000000ull);
> > +    Trace("TIME SINCE BOOT: Seconds = %llu NanoSeconds = %llu\n",
> > +          SystemTime / 1000000000ull,
> > +          SystemTime % 1000000000ull);
> >
> >      // Convert wallclock from Unix epoch (1970) to Windows epoch (1601)
> >      Seconds += 11644473600ull;
> >
> >      // Add in time since host boot
> > -    Seconds += Now.QuadPart / 1000000000ull;
> > -    NanoSeconds += Now.QuadPart % 1000000000ull;
> > +    Seconds += SystemTime / 1000000000ull;
> > +    NanoSeconds += SystemTime % 1000000000ull;
> >
> > -    // Converto to system time format
> > +    // Convert to system time format
> >      Now.QuadPart = (Seconds * 10000000ull) + (NanoSeconds / 100ull);
> >
> >      RtlTimeToTimeFields(&Now, &Time);
> > --
> > 2.10.1.windows.1
> >
> >
> > _______________________________________________
> > win-pv-devel mailing list
> > win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> > https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel