Re: [Xen-devel] [PATCH v9] new config option vtsc_tolerance_khz to avoid TSC emulation [and 1 more messages]

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Jan Beulich writes ("Re: [PATCH v9] new config option vtsc_tolerance_khz to 
avoid TSC emulation"):
> Problem is - discussion around this was (iirc) happening not only on
> the list, but also on irc (including perhaps private chats).

Hrm.  Well, if it didn't happen on the list, it didn't happen.  It is
often useful and productive, of course, to thrash something out on
irc, or ping there, or whatever but:

Conclusions from irc (and from in-person conversations, phone calls,
or other kinds of un-minuted discussions) must be transferred to email
as otherwise they are lost (and the effort of having them is often
wasted as the conversation has to be had again).

If properly writing up the conclusion from an irc conversation is too
hard, pasting a transcript into an email seems a bare minimum.

> It was for that reason that I made my R-b conditional upon Andrew at
> least giving an informal go-ahead (otherwise, together with Wei's
> R-b, the patch could have gone in).

Right.  Thanks for the clarification.


Andrew Cooper writes ("Re: [PATCH v9] new config option vtsc_tolerance_khz to 
avoid TSC emulation"):
> [lots of stuff]

Thanks for the review.  I hope Olaf will be able to address most of
it, but:

Andrew:
> [Olaf:]
> > Existing padding fields are reused to store vtsc_khz_tolerance as u16.
> > The padding is sent as zero in write_tsc_info to the receving host.
> > The padding is undefined if the changed code runs as receiver.
> 
> I'm not sure what you mean by this final sentence.

Me neither.

> > handle_tsc_info has no code to verify that padding is indeed zero. Due
> > to the lack of a version field it is impossible to know if the sender
> > already has the newly introduced vtsc_tolerance field. In the worst
> > case the receiving domU will get an unemulated TSC.
> 
> The lack of padding verification is deliberate, for forwards
> compatibility.  Why does the sending code matter?  One way or another,
> if the field is 0, the option wasn't present or wasn't configured. 
> Neither of these situations affect the decision-making that the
> receiving side needs to perform.

We are talking here about an unused field that is (supposedly?)
always sent as zero ?

AIUI:

In the new code the semantics of zero is "do not allow any tolerance".
The old code handles this correctly.  The issue is with migrations
from new code to old: the tolerance value will silently ignored.

Presumably this is considered preferable to the alternative, which is
to extend the migration stream in a deliberately incompatible way so
that the migration fails.  I think this is what Olaf is trying to
say ?

> > Signed-off-by: Olaf Hering <olaf@xxxxxxxxx>
> > Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx> (v07/v08)
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> (v08)
> 
> I'm still -0.5 for this patch.  I can appreciate why you want it, but it
> is a gross hack which only works when you don't skew time more than NTP
> in the guest can cope with.

That surely is why there is a limit to the tolerance.  I've asked Olaf
to try to quantify an appropriate limit.

>   My gut feeling is that there will be other
> more subtle fallout.

That's not particularly convincing to me.  But if you could try to be
more specific I think this could be usefull addressed in the
documentation for the feature ?


Olaf, I think the ball is now in your court.  I hope you can address
Andrew's comments, and maybe mine too.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel