Virtualization - Mar 2022 - [PATCH V2 03/11] perf/x86: Add support for TSC in nanoseconds as a perf event clock

On 07.03.22 10:50, Peter Zijlstra wrote:
> On Fri, Mar 04, 2022 at 08:27:45PM +0200, Adrian Hunter wrote:
>> On 04/03/2022 15:41, Peter Zijlstra wrote:
>>> On Mon, Feb 14, 2022 at 01:09:06PM +0200, Adrian Hunter wrote:
>>>> Currently, when Intel PT is used within a VM guest, it is not
possible to
>>>> make use of TSC because perf clock is subject to
paravirtualization.
>>>
>>> Yeah, so how much of that still makes sense, or ever did? AFAIK the
>>> whole pv_clock thing is utter crazy. Should we not fix that
instead?
>>
>> Presumably pv_clock must work with different host operating systems.
>> Similarly, KVM must work with different guest operating systems.
>> Perhaps I'm wrong, but I imagine re-engineering time virtualization
>> might be a pretty big deal,  far exceeding the scope of these patches.
> 
> I think not; on both counts. That is, I don't think it's going to
be
> hard, and even it if were, it would still be the right thing to do.
> 
> We're not going to add interface just to work around a known broken
> piece of crap just because we don't want to fix it.
> 
> So I'm thinking we should do the below and simply ignore any paravirt
> sched clock offered when there's ART on.
> 
> ---
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index 4420499f7bb4..a1f179ed39bf 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -145,6 +145,15 @@ DEFINE_STATIC_CALL(pv_sched_clock,
native_sched_clock);
>   
>   void paravirt_set_sched_clock(u64 (*func)(void))
>   {
> +	/*
> +	 * Anything with ART on promises to have sane TSC, otherwise the whole
> +	 * ART thing is useless. In order to make ART useful for guests, we
> +	 * should continue to use the TSC. As such, ignore any paravirt
> +	 * muckery.
> +	 */
> +	if (cpu_feature_enabled(X86_FEATURE_ART))
> +		return;
> +
>   	static_call_update(pv_sched_clock, func);
>   }
>   
> 
NAK, this will break live migration of a guest coming from a host
without this feature.


Juergen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xB0DE9DD628BF132F.asc
Type: application/pgp-keys
Size: 3098 bytes
Desc: OpenPGP public key
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20220307/f58d5b0a/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20220307/f58d5b0a/attachment.sig>

On Mon, Mar 07, 2022 at 11:06:46AM +0100, Juergen Gross wrote:
> > diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> > index 4420499f7bb4..a1f179ed39bf 100644
> > --- a/arch/x86/kernel/paravirt.c
> > +++ b/arch/x86/kernel/paravirt.c
> > @@ -145,6 +145,15 @@ DEFINE_STATIC_CALL(pv_sched_clock,
native_sched_clock);
> >   void paravirt_set_sched_clock(u64 (*func)(void))
> >   {
> > +	/*
> > +	 * Anything with ART on promises to have sane TSC, otherwise the
whole
> > +	 * ART thing is useless. In order to make ART useful for guests, we
> > +	 * should continue to use the TSC. As such, ignore any paravirt
> > +	 * muckery.
> > +	 */
> > +	if (cpu_feature_enabled(X86_FEATURE_ART))
> > +		return;
> > +
> >   	static_call_update(pv_sched_clock, func);
> >   }
> > 
> 
> NAK, this will break live migration of a guest coming from a host
> without this feature.
I thought the whole live-migration nonsense made sure to equalize crud
like that. That is, then don't expose ART to the guest.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20220307/6fb65d3b/attachment-0001.sig>