Re: [Xen-devel] [xen-unstable test] 18092: tolerable FAIL

[Tim Deegan <tim@xxxxxxx>]

At 11:11 +0100 on 10 Jun (1370862717), Jan Beulich wrote:
> >>> On 10.06.13 at 11:52, Tim Deegan <tim@xxxxxxx> wrote:
> > At 08:52 +0100 on 07 Jun (1370595174), Jan Beulich wrote:
> >> >>> On 07.06.13 at 08:47, xen.org <ian.jackson@xxxxxxxxxxxxx> wrote:
> >> > flight 18092 xen-unstable real [real]
> >> > http://www.chiark.greenend.org.uk/~xensrcts/logs/18092/ 
> >> > 
> >> > Failures :-/ but no regressions.
> >> > 
> >> > Tests which are failing intermittently (not blocking):
> >> >  test-amd64-amd64-xl-qemuu-winxpsp3  8 guest-saverestore     fail pass 
> >> > in 18090
> >> 
> >> So commit eb60be3dd870aecfa47bed1118069680389c15f7 ("x86:
> >> don't pass negative time to gtime_to_gtsc()") caught something
> >> here after the first reboot of the Windows install in the guest:
> >> 
> >> Jun  7 02:35:44.623032 (XEN) d2v0: bogus time -19766120 (offsets 
> > -362881846364/0)
> >> 
> >> (and many more instances of this during the following about 1.5 sec).
> >> 
> >> Looking at the involved code again, I realize that pl->stime_offset
> >> gets set from calling get_s_time(), yet the calculation in
> >> __update_vcpu_system_time() starts from
> >> this_cpu(cpu_time).stime_local_stamp, which validly can be before
> >> the value the initializing get_s_time() invocation returned. So stime
> >> can validly be negative here, and calculating tsc_stamp based on
> >> the flushed-to-zero stime value is incorrect (and we really ought to
> >> set tsc_timestamp to a value wrapped downwards through zero -
> >> question is whether all possible guest calculations would cope with
> >> that - Linux'es clearly would).
> > 
> > Hmm.  The calculation specified in the public header will work: it uses
> > plain subtraction on 64-bit unsigned integers.  So for once we can claim
> > that the ABI is documented on this point. :)  
> > 
> > But wait -- this is in an 'is_hvm_domain' block.  I thought PV drivers
> > in HVM guests used HVMOP_get_time rather than calculating NOW()
> > themselves, because they don't know the TSC offset.  Or is that only on
> > Windows, where the TSC is controlled by non-PV parts of the kernel?
> > 
> > Either way, fixing gtime_to_gtsc() to handle stime < 0 sounds right.
> 
> Actually, I don't think that would be the proper course of action -
> I continue to think that this function should only be called with
> non-negative (i.e. unsigned) deltas. Instead I think the caller should
> take care of calling it with the negated stime, and then doing with
> the result whatever is appropriate

OK, that makes sense - I guess this is the only caller that would ever
need to handle negative offsets.

Tim.

> - the question was whether we
> can assume that users can deal with the effectively underflowed
> TSC stamp that wpuld result here. If, as you say, we take what the
> public header has as ABI specification, then we can safely assume so.
> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel