[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 06/13] x86/hvm: Scale host TSC when setting/getting guest TSC



On 10/27/2015 09:10 AM, Boris Ostrovsky wrote:
On 10/27/2015 04:44 AM, Haozhong Zhang wrote:
On Thu, Oct 22, 2015 at 08:17:29AM -0600, Jan Beulich wrote:
On 28.09.15 at 09:13, <haozhong.zhang@xxxxxxxxx> wrote:
The existing hvm_set_guest_tsc_fixed() and hvm_get_guest_tsc_fixed()
calculate the guest TSC by adding the TSC offset to the host TSC. When
the TSC scaling is enabled, the host TSC should be scaled first. This
patch adds the scaling logic to those two functions.
Just like mentioned for the first twp patches - I'd first of all like to
understand why the lack of scaling this wasn't an issue for SVM so
far. What you reads plausible, but assuming that SVM TSC scaling
code was tested, I'm hesitant to apply changes to it without
understanding the details (or at least without SVM maintainers'
consent).

Hi SVM maintainers,

Could you help to review this patch 6 as well as patch 2? They intend
to fix bugs in SVM TSC ratio code (or code that affects SVM TSC ratio
code).

The detailed explanations of patch 2 and patch 6 can be found at
http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg01490.html
and
http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02843.html
respectively.

I agree with patch 2 (so you can add my Reviewed-by).


but I am not so sure about patch 6 (and 11, together with existing SVM handlers).


I don't have latest Intel's manual handy but for SVM the guest TSC value is calculated as scaled host TSC plus *unscaled* VMCB's TSC offset. Is Intel's implementation similar?

Both svm_set_tsc_offset and vmx_set_tsc_offset (as proposed in patch 11) write VMCB/VMCS with guest (i.s. scaled) offset, and that doesn't seem right.

(I seem to have lost Haozhong on my previous email)

No, it is right (or, rather, since TSC offset is a constant it can be used as scaled or unscaled, depending on how you want to implement it).

So if you adjust patch 11 (and corresponding SVM code) to take scaling out from there this patch would be correct.

Of course I still can't test this since the two machines that I have available are running at fairly close frequencies.


-boris




If I am right then I think (1) SVM is broken now and (2) patches 6 and 11 don't fix this brokenness and instead propagate it to VMX.

(I should have thought about this when I last replied to you asking to move scaling out of vmx_set_tsc_offset(). But I re-read this code now and it doesn't make sense to me anymore).

-boris



Thanks,
Haozhong

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -388,13 +388,12 @@ void hvm_set_guest_tsc_fixed(struct vcpu *v, u64 guest_tsc, u64 at_tsc)
          tsc = hvm_get_guest_time_fixed(v, at_tsc);
          tsc = gtime_to_gtsc(v->domain, tsc);
      }
-    else if ( at_tsc )
-    {
-        tsc = at_tsc;
-    }
      else
      {
-        tsc = rdtsc();
+        tsc = at_tsc ? at_tsc : rdtsc();
In cases like this please prefer the gcc extension allowing the middle
operand of the ?: to be omitted.

Jan



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


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.