diff mbox

[v3,02/13] x86/time.c: Use correct guest TSC frequency in tsc_get_info()

Message ID 1451531020-29964-3-git-send-email-haozhong.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Haozhong Zhang Dec. 31, 2015, 3:03 a.m. UTC
When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
frequency. However, tsc_set_info() may set the guest TSC frequency to a
value different than the host. In order to keep consistent to
tsc_set_info(), this patch makes tsc_get_info() use the value set by
tsc_set_info() as the guest TSC frequency.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Changes in v3:
 (addressing Boris Ostrovsky's comments)
 * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.

 xen/arch/x86/time.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Boris Ostrovsky Jan. 4, 2016, 5:48 p.m. UTC | #1
On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
> TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
> tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
> frequency. However, tsc_set_info() may set the guest TSC frequency to a
> value different than the host. In order to keep consistent to
> tsc_set_info(), this patch makes tsc_get_info() use the value set by
> tsc_set_info() as the guest TSC frequency.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> Changes in v3:
>   (addressing Boris Ostrovsky's comments)
>   * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
>
>   xen/arch/x86/time.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index 0059b6a..d83f068 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>                     uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
>                     uint32_t *incarnation)
>   {
> +    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
> +                                cpu_has_tsc_ratio;
> +

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

although I still think that having !d->arch.vtsc would be better, even 
if not strictly required.



>       *incarnation = d->arch.incarnation;
>       *tsc_mode = d->arch.tsc_mode;
>   
> @@ -1769,7 +1772,7 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>           }
>           tsc = rdtsc();
>           *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns);
> -        *gtsc_khz = cpu_khz;
> +        *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : cpu_khz;
>           break;
>       case TSC_MODE_PVRDTSCP:
>           if ( d->arch.vtsc )
> @@ -1780,9 +1783,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>           else
>           {
>               tsc = rdtsc();
> -            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
> +            *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
>                               d->arch.vtsc_offset;
> -            *gtsc_khz = 0; /* ignored by tsc_set_info */
> +            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;
>           }
>           break;
>       }
Haozhong Zhang Jan. 5, 2016, 12:32 a.m. UTC | #2
On 01/04/16 12:48, Boris Ostrovsky wrote:
> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> >When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
> >TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
> >tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
> >frequency. However, tsc_set_info() may set the guest TSC frequency to a
> >value different than the host. In order to keep consistent to
> >tsc_set_info(), this patch makes tsc_get_info() use the value set by
> >tsc_set_info() as the guest TSC frequency.
> >
> >Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >---
> >Changes in v3:
> >  (addressing Boris Ostrovsky's comments)
> >  * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
> >
> >  xen/arch/x86/time.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> >diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> >index 0059b6a..d83f068 100644
> >--- a/xen/arch/x86/time.c
> >+++ b/xen/arch/x86/time.c
> >@@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> >                    uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
> >                    uint32_t *incarnation)
> >  {
> >+    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
> >+                                cpu_has_tsc_ratio;
> >+
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> although I still think that having !d->arch.vtsc would be better, even if
> not strictly required.
>
I'll add in the next version.

Haozhong
Jan Beulich Jan. 8, 2016, 9:05 a.m. UTC | #3
>>> On 04.01.16 at 18:48, <boris.ostrovsky@oracle.com> wrote:
> On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
>> When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
>> TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
>> tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
>> frequency. However, tsc_set_info() may set the guest TSC frequency to a
>> value different than the host. In order to keep consistent to
>> tsc_set_info(), this patch makes tsc_get_info() use the value set by
>> tsc_set_info() as the guest TSC frequency.
>>
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
>> ---
>> Changes in v3:
>>   (addressing Boris Ostrovsky's comments)
>>   * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
>>
>>   xen/arch/x86/time.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
>> index 0059b6a..d83f068 100644
>> --- a/xen/arch/x86/time.c
>> +++ b/xen/arch/x86/time.c
>> @@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>>                     uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
>>                     uint32_t *incarnation)
>>   {
>> +    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
>> +                                cpu_has_tsc_ratio;
>> +
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> although I still think that having !d->arch.vtsc would be better, even 
> if not strictly required.

Since Haozhong agreed, and for symmetry with patch 1 I'm going to
commit with this added, plus ...

>> @@ -1780,9 +1783,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
>>           else
>>           {
>>               tsc = rdtsc();
>> -            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
>> +            *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
>>                               d->arch.vtsc_offset;
>> -            *gtsc_khz = 0; /* ignored by tsc_set_info */
>> +            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;

... with the comment here retained.

Jan
Haozhong Zhang Jan. 8, 2016, 9:12 a.m. UTC | #4
On 01/08/16 02:05, Jan Beulich wrote:
> >>> On 04.01.16 at 18:48, <boris.ostrovsky@oracle.com> wrote:
> > On 12/30/2015 10:03 PM, Haozhong Zhang wrote:
> >> When the TSC mode of a HVM container is TSC_MODE_DEFAULT or
> >> TSC_MODE_PVRDTSCP and no TSC emulation is used, the existing
> >> tsc_get_info() uses the host TSC frequency (cpu_khz) as the guest TSC
> >> frequency. However, tsc_set_info() may set the guest TSC frequency to a
> >> value different than the host. In order to keep consistent to
> >> tsc_set_info(), this patch makes tsc_get_info() use the value set by
> >> tsc_set_info() as the guest TSC frequency.
> >>
> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> >> ---
> >> Changes in v3:
> >>   (addressing Boris Ostrovsky's comments)
> >>   * Use this_cpu(cpu_time).tsc_scale for both scaling and non-scaling cases.
> >>
> >>   xen/arch/x86/time.c | 9 ++++++---
> >>   1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> >> index 0059b6a..d83f068 100644
> >> --- a/xen/arch/x86/time.c
> >> +++ b/xen/arch/x86/time.c
> >> @@ -1749,6 +1749,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> >>                     uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
> >>                     uint32_t *incarnation)
> >>   {
> >> +    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
> >> +                                cpu_has_tsc_ratio;
> >> +
> > 
> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > 
> > although I still think that having !d->arch.vtsc would be better, even 
> > if not strictly required.
> 
> Since Haozhong agreed, and for symmetry with patch 1 I'm going to
> commit with this added, plus ...
> 
> >> @@ -1780,9 +1783,9 @@ void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
> >>           else
> >>           {
> >>               tsc = rdtsc();
> >> -            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
> >> +            *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
> >>                               d->arch.vtsc_offset;
> >> -            *gtsc_khz = 0; /* ignored by tsc_set_info */
> >> +            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;
> 
> ... with the comment here retained.
>
And it's better to refine the comment like
/* ignored by tsc_set_info if TSC scaling disabled */

Haozhong
diff mbox

Patch

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 0059b6a..d83f068 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1749,6 +1749,9 @@  void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
                   uint64_t *elapsed_nsec, uint32_t *gtsc_khz,
                   uint32_t *incarnation)
 {
+    bool_t enable_tsc_scaling = has_hvm_container_domain(d) &&
+                                cpu_has_tsc_ratio;
+
     *incarnation = d->arch.incarnation;
     *tsc_mode = d->arch.tsc_mode;
 
@@ -1769,7 +1772,7 @@  void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
         }
         tsc = rdtsc();
         *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns);
-        *gtsc_khz = cpu_khz;
+        *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : cpu_khz;
         break;
     case TSC_MODE_PVRDTSCP:
         if ( d->arch.vtsc )
@@ -1780,9 +1783,9 @@  void tsc_get_info(struct domain *d, uint32_t *tsc_mode,
         else
         {
             tsc = rdtsc();
-            *elapsed_nsec = scale_delta(tsc, &d->arch.vtsc_to_ns) -
+            *elapsed_nsec = scale_delta(tsc, &this_cpu(cpu_time).tsc_scale) -
                             d->arch.vtsc_offset;
-            *gtsc_khz = 0; /* ignored by tsc_set_info */
+            *gtsc_khz = enable_tsc_scaling ? d->arch.tsc_khz : 0;
         }
         break;
     }