All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/time: Don't use virtual TSC if host and guest frequencies are equal
@ 2017-03-15 19:48 Boris Ostrovsky
  2017-03-16 10:40 ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Boris Ostrovsky @ 2017-03-15 19:48 UTC (permalink / raw)
  To: xen-devel; +Cc: andrew.cooper3, boris.ostrovsky, jbeulich

Commit 82713ec8d2 ("x86: use native RDTSC(P) execution when guest and
host frequencies are the same") left out optimization for PV guests
when host and guest run at the same frequency.

For such a case we should be able not to use virtual TSC regardless
of whether we are runing before or after a migration (i.e. regardless
of incarnation value).

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Suggested-by: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/time.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index faa638b..1a13f2f 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -2051,17 +2051,11 @@ void tsc_set_info(struct domain *d,
         d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
         d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
         set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
-        /*
-         * In default mode use native TSC if the host has safe TSC and:
-         *  HVM/PVH: host and guest frequencies are the same (either
-         *           "naturally" or via TSC scaling)
-         *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
-         */
+
         if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
-             (has_hvm_container_domain(d) ?
-              (d->arch.tsc_khz == cpu_khz ||
-               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz)) :
-              incarnation == 0) )
+             (d->arch.tsc_khz == cpu_khz || incarnation == 0 ||
+              (has_hvm_container_domain(d) &&
+               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz))) )
         {
     case TSC_MODE_NEVER_EMULATE:
             d->arch.vtsc = 0;
-- 
1.8.3.1


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/time: Don't use virtual TSC if host and guest frequencies are equal
  2017-03-15 19:48 [PATCH] x86/time: Don't use virtual TSC if host and guest frequencies are equal Boris Ostrovsky
@ 2017-03-16 10:40 ` Jan Beulich
  2017-03-16 12:44   ` Boris Ostrovsky
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2017-03-16 10:40 UTC (permalink / raw)
  To: Boris Ostrovsky; +Cc: andrew.cooper3, xen-devel

>>> On 15.03.17 at 20:48, <boris.ostrovsky@oracle.com> wrote:
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -2051,17 +2051,11 @@ void tsc_set_info(struct domain *d,
>          d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
>          d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
>          set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
> -        /*
> -         * In default mode use native TSC if the host has safe TSC and:
> -         *  HVM/PVH: host and guest frequencies are the same (either
> -         *           "naturally" or via TSC scaling)
> -         *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
> -         */
> +
>          if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
> -             (has_hvm_container_domain(d) ?
> -              (d->arch.tsc_khz == cpu_khz ||
> -               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz)) :
> -              incarnation == 0) )
> +             (d->arch.tsc_khz == cpu_khz || incarnation == 0 ||

Is the incarnation comparison really needed here, i.e. doesn't it
being zero imply the two frequencies to match in default mode?

Jan


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/time: Don't use virtual TSC if host and guest frequencies are equal
  2017-03-16 10:40 ` Jan Beulich
@ 2017-03-16 12:44   ` Boris Ostrovsky
  2017-03-16 13:31     ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Boris Ostrovsky @ 2017-03-16 12:44 UTC (permalink / raw)
  To: Jan Beulich; +Cc: andrew.cooper3, xen-devel

On 03/16/2017 06:40 AM, Jan Beulich wrote:
>>>> On 15.03.17 at 20:48, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -2051,17 +2051,11 @@ void tsc_set_info(struct domain *d,
>>          d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
>>          d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
>>          set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
>> -        /*
>> -         * In default mode use native TSC if the host has safe TSC and:
>> -         *  HVM/PVH: host and guest frequencies are the same (either
>> -         *           "naturally" or via TSC scaling)
>> -         *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
>> -         */
>> +
>>          if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
>> -             (has_hvm_container_domain(d) ?
>> -              (d->arch.tsc_khz == cpu_khz ||
>> -               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz)) :
>> -              incarnation == 0) )
>> +             (d->arch.tsc_khz == cpu_khz || incarnation == 0 ||
> Is the incarnation comparison really needed here, i.e. doesn't it
> being zero imply the two frequencies to match in default mode?

It is not necessary but I wanted to keep it for clarity so that it is
explicit that when a domain is born we don't use vtsc.


-boris


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/time: Don't use virtual TSC if host and guest frequencies are equal
  2017-03-16 12:44   ` Boris Ostrovsky
@ 2017-03-16 13:31     ` Jan Beulich
  2017-03-16 14:22       ` Boris Ostrovsky
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2017-03-16 13:31 UTC (permalink / raw)
  To: Boris Ostrovsky; +Cc: andrew.cooper3, xen-devel

>>> On 16.03.17 at 13:44, <boris.ostrovsky@oracle.com> wrote:
> On 03/16/2017 06:40 AM, Jan Beulich wrote:
>>>>> On 15.03.17 at 20:48, <boris.ostrovsky@oracle.com> wrote:
>>> --- a/xen/arch/x86/time.c
>>> +++ b/xen/arch/x86/time.c
>>> @@ -2051,17 +2051,11 @@ void tsc_set_info(struct domain *d,
>>>          d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
>>>          d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
>>>          set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
>>> -        /*
>>> -         * In default mode use native TSC if the host has safe TSC and:
>>> -         *  HVM/PVH: host and guest frequencies are the same (either
>>> -         *           "naturally" or via TSC scaling)
>>> -         *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
>>> -         */
>>> +
>>>          if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
>>> -             (has_hvm_container_domain(d) ?
>>> -              (d->arch.tsc_khz == cpu_khz ||
>>> -               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz)) :
>>> -              incarnation == 0) )
>>> +             (d->arch.tsc_khz == cpu_khz || incarnation == 0 ||
>> Is the incarnation comparison really needed here, i.e. doesn't it
>> being zero imply the two frequencies to match in default mode?
> 
> It is not necessary but I wanted to keep it for clarity so that it is
> explicit that when a domain is born we don't use vtsc.

Well, considering the history here, I think its presence is rather
going to raise questions than to answer any, so if anything I'd
suggest to have an ASSERT() to that effect, at once serving as
sort of documentation. But I may be the only one thinking this
way ...

Jan


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/time: Don't use virtual TSC if host and guest frequencies are equal
  2017-03-16 13:31     ` Jan Beulich
@ 2017-03-16 14:22       ` Boris Ostrovsky
  2017-03-16 15:34         ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Boris Ostrovsky @ 2017-03-16 14:22 UTC (permalink / raw)
  To: Jan Beulich; +Cc: andrew.cooper3, xen-devel

On 03/16/2017 09:31 AM, Jan Beulich wrote:
>>>> On 16.03.17 at 13:44, <boris.ostrovsky@oracle.com> wrote:
>> On 03/16/2017 06:40 AM, Jan Beulich wrote:
>>>>>> On 15.03.17 at 20:48, <boris.ostrovsky@oracle.com> wrote:
>>>> --- a/xen/arch/x86/time.c
>>>> +++ b/xen/arch/x86/time.c
>>>> @@ -2051,17 +2051,11 @@ void tsc_set_info(struct domain *d,
>>>>          d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
>>>>          d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
>>>>          set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
>>>> -        /*
>>>> -         * In default mode use native TSC if the host has safe TSC and:
>>>> -         *  HVM/PVH: host and guest frequencies are the same (either
>>>> -         *           "naturally" or via TSC scaling)
>>>> -         *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
>>>> -         */
>>>> +
>>>>          if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
>>>> -             (has_hvm_container_domain(d) ?
>>>> -              (d->arch.tsc_khz == cpu_khz ||
>>>> -               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz)) :
>>>> -              incarnation == 0) )
>>>> +             (d->arch.tsc_khz == cpu_khz || incarnation == 0 ||
>>> Is the incarnation comparison really needed here, i.e. doesn't it
>>> being zero imply the two frequencies to match in default mode?
>> It is not necessary but I wanted to keep it for clarity so that it is
>> explicit that when a domain is born we don't use vtsc.
> Well, considering the history here, I think its presence is rather
> going to raise questions than to answer any, so if anything I'd
> suggest to have an ASSERT() to that effect, at once serving as
> sort of documentation. But I may be the only one thinking this
> way ...


Haven't thought about an ASSERT. I can do that. Something like

ASSERT(gtsc_khz == 0 ? incarnation == 0 : true);

-boris


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/time: Don't use virtual TSC if host and guest frequencies are equal
  2017-03-16 14:22       ` Boris Ostrovsky
@ 2017-03-16 15:34         ` Jan Beulich
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2017-03-16 15:34 UTC (permalink / raw)
  To: Boris Ostrovsky; +Cc: andrew.cooper3, xen-devel

>>> On 16.03.17 at 15:22, <boris.ostrovsky@oracle.com> wrote:
> On 03/16/2017 09:31 AM, Jan Beulich wrote:
>>>>> On 16.03.17 at 13:44, <boris.ostrovsky@oracle.com> wrote:
>>> On 03/16/2017 06:40 AM, Jan Beulich wrote:
>>>>>>> On 15.03.17 at 20:48, <boris.ostrovsky@oracle.com> wrote:
>>>>> --- a/xen/arch/x86/time.c
>>>>> +++ b/xen/arch/x86/time.c
>>>>> @@ -2051,17 +2051,11 @@ void tsc_set_info(struct domain *d,
>>>>>          d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
>>>>>          d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
>>>>>          set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
>>>>> -        /*
>>>>> -         * In default mode use native TSC if the host has safe TSC and:
>>>>> -         *  HVM/PVH: host and guest frequencies are the same (either
>>>>> -         *           "naturally" or via TSC scaling)
>>>>> -         *  PV: guest has not migrated yet (and thus arch.tsc_khz == 
> cpu_khz)
>>>>> -         */
>>>>> +
>>>>>          if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
>>>>> -             (has_hvm_container_domain(d) ?
>>>>> -              (d->arch.tsc_khz == cpu_khz ||
>>>>> -               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz)) :
>>>>> -              incarnation == 0) )
>>>>> +             (d->arch.tsc_khz == cpu_khz || incarnation == 0 ||
>>>> Is the incarnation comparison really needed here, i.e. doesn't it
>>>> being zero imply the two frequencies to match in default mode?
>>> It is not necessary but I wanted to keep it for clarity so that it is
>>> explicit that when a domain is born we don't use vtsc.
>> Well, considering the history here, I think its presence is rather
>> going to raise questions than to answer any, so if anything I'd
>> suggest to have an ASSERT() to that effect, at once serving as
>> sort of documentation. But I may be the only one thinking this
>> way ...
> 
> 
> Haven't thought about an ASSERT. I can do that. Something like
> 
> ASSERT(gtsc_khz == 0 ? incarnation == 0 : true);

ASSERT(incarnation || d->arch.tsc_khz == cpu_khz);

Jan


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-03-16 15:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-15 19:48 [PATCH] x86/time: Don't use virtual TSC if host and guest frequencies are equal Boris Ostrovsky
2017-03-16 10:40 ` Jan Beulich
2017-03-16 12:44   ` Boris Ostrovsky
2017-03-16 13:31     ` Jan Beulich
2017-03-16 14:22       ` Boris Ostrovsky
2017-03-16 15:34         ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.