Re: [Xen-devel] Ping: [PATCH v2 RESEND] x86/time: correctly honor late clearing of TSC related feature flags

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.12.16 at 12:04, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 15/12/16 09:49, Jan Beulich wrote:
>>>>> On 06.12.16 at 11:51, <JBeulich@xxxxxxxx> wrote:
>>> As such clearing of flags may have an impact on the selected rendezvous
>>> function, handle such in a central place.
>>>
>>> But don't allow such feature flags to be cleared during CPU hotplug:
>>> Platform and local system times may have diverged significantly by
>>> then, potentially causing noticably (even if only temporary) strange
>>> behavior. As we're anyway expecting only sufficiently similar CPUs to
>>> appear during hotplug, this shouldn't be introducing new limitations.
>>>
>>> Reported-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> Tested-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
>>> Tested-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>>> ---
>>> The resend is mainly to get the discussion going again on what the
>>> alternatives are, if this patch is not acceptable.
>> Even if you don't agree with the patch, can we at least revive
>> the discussion of what alternatives there are?
> 
> Sorry - it slipped through the cracks.  I have no issue with the
> principle of the patch.
> 
> The only problem I have, which we didn't sort out last time, is the
> initialisation of rendezvous_fn
> 
> It is still my opinion that under no circumstance is it ok for
> clear_tsc_cap() to modify time_calibration_rendezvous_fn from
> time_calibration_tsc_rendezvous to time_calibration_std_rendezvous,
> which can in principle happen because rendezvous_fn doesn't get
> initialised from the current time_calibration_rendezvous_fn.

I understand that this is your primary reservation against the patch,
yet at the same time this is also the purpose of the patch. If we're
not allowed to change the rendezvous function to what it is supposed
to be given the final set of CPU features we determined, then the
whole patch is pointless. At the time we first set the pointer, we
simply don't know yet what we'll find once we brought up APs. If we
knew, we'd set it differently. Since pre- and post-AP bringup
knowledge will always have the potential to differ, we need to adjust
the function pointer in one direction anyway: Either we set it to std
early, and switch to tsc later, or we allow setting it to tsc early,
reverting to std if need be.

Jan


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