[PATCH 01/02] pvrdtscp: move the place of write_rdtscp_aux()

[PATCH 01/02] pvrdtscp: move the place of write_rdtscp_aux()

[Xu, Dongxiao]

[-- Attachment #1: Type: text/plain, Size: 425 bytes --]

 Pvrdtscp: move write_rdtscp_aux() to paravirt_ctxt_switch_to()
 - Currently write_rdtscp_aux() is placed in update_vcpu_system_time(), which
is called by schedule() before context_switch(). This will break the HVM guest
TSC_AUX state because at this point, MSR hasn't beed saved for HVM guests.So
put the function in the point when a PV vcpu is really scheduled in. 

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>

[-- Attachment #2: 01-write-pvrdtscp-tsc-aux.patch --]
[-- Type: application/octet-stream, Size: 1624 bytes --]

Pvrdtscp: move write_rdtscp_aux() to paravirt_ctxt_switch_to()
 - Currently write_rdtscp_aux() is placed in update_vcpu_system_time(), which
is called by schedule() before context_switch(). This will break the HVM guest
TSC_AUX state because at this point, MSR hasn't beed saved for HVM guests.So
put the function in the point when a PV vcpu is really scheduled in. 

Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>

diff -r 7f611de6b93c xen/arch/x86/domain.c
--- a/xen/arch/x86/domain.c	Tue Dec 08 14:14:27 2009 +0000
+++ b/xen/arch/x86/domain.c	Fri Dec 11 12:02:49 2009 +0800
@@ -1298,6 +1298,10 @@ static void paravirt_ctxt_switch_to(stru
         write_debugreg(6, v->arch.guest_context.debugreg[6]);
         write_debugreg(7, v->arch.guest_context.debugreg[7]);
     }
+
+    if ( (v->domain->arch.tsc_mode ==  TSC_MODE_PVRDTSCP) &&
+         boot_cpu_has(X86_FEATURE_RDTSCP) )
+        write_rdtscp_aux(v->domain->arch.incarnation);
 }
 
 /* Update per-VCPU guest runstate shared memory area (if registered). */
diff -r 7f611de6b93c xen/arch/x86/time.c
--- a/xen/arch/x86/time.c	Tue Dec 08 14:14:27 2009 +0000
+++ b/xen/arch/x86/time.c	Fri Dec 11 12:02:49 2009 +0800
@@ -831,10 +831,6 @@ static void __update_vcpu_system_time(st
                                 &d->arch.ns_to_vtsc);
     else
         tsc_stamp = t->local_tsc_stamp;
-
-    if ( (d->arch.tsc_mode ==  TSC_MODE_PVRDTSCP) &&
-         boot_cpu_has(X86_FEATURE_RDTSCP) )
-        write_rdtscp_aux(d->arch.incarnation);
 
     /* Don't bother unless timestamps have changed or we are forced. */
     if ( !force && (u->tsc_timestamp == tsc_stamp) )


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH 01/02] pvrdtscp: move the place of write_rdtscp_aux()

[Dan Magenheimer]

This patch is wrong for TSC_MODE_PVRDTSCP.  The original
code should be left in __update_vcpu_system_time and
the context switch code should write to TSC_AUX only
if tsc_mode is *NOT* TSC_MODE_PVRDTSCP because the
implementation of full rdtscp functionality and
TSC_MODE_PVRDTSCP mode are incompatible.

But I continue to question if this should be done at all.
See other thread.

> -----Original Message-----
> From: Xu, Dongxiao [mailto:dongxiao.xu@intel.com]
> Sent: Friday, December 11, 2009 4:54 AM
> To: xen-devel@lists.xensource.com
> Cc: Keir Fraser
> Subject: [Xen-devel][PATCH 01/02] pvrdtscp: move the place of
> write_rdtscp_aux()
> 
> 
>  Pvrdtscp: move write_rdtscp_aux() to paravirt_ctxt_switch_to()
>  - Currently write_rdtscp_aux() is placed in 
> update_vcpu_system_time(), which
> is called by schedule() before context_switch(). This will 
> break the HVM guest
> TSC_AUX state because at this point, MSR hasn't beed saved 
> for HVM guests.So
> put the function in the point when a PV vcpu is really scheduled in. 
> 
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>

RE: [PATCH 01/02] pvrdtscp: move the place of write_rdtscp_aux()

[Xu, Dongxiao]

Dan Magenheimer wrote:
> This patch is wrong for TSC_MODE_PVRDTSCP.  The original
> code should be left in __update_vcpu_system_time and
> the context switch code should write to TSC_AUX only
> if tsc_mode is *NOT* TSC_MODE_PVRDTSCP because the
> implementation of full rdtscp functionality and
> TSC_MODE_PVRDTSCP mode are incompatible.

Could you detail the wrong place? Appreciate for this!
In my patch, I have done save/restore for guest/host 
TSC_AUX MSR, so it will not break PVRDTSC logic. 
 
> 
> But I continue to question if this should be done at all.
> See other thread.
> 
>> -----Original Message-----
>> From: Xu, Dongxiao [mailto:dongxiao.xu@intel.com]
>> Sent: Friday, December 11, 2009 4:54 AM
>> To: xen-devel@lists.xensource.com
>> Cc: Keir Fraser
>> Subject: [Xen-devel][PATCH 01/02] pvrdtscp: move the place of
>> write_rdtscp_aux() 
>> 
>> 
>>  Pvrdtscp: move write_rdtscp_aux() to paravirt_ctxt_switch_to()
>>  - Currently write_rdtscp_aux() is placed in
>> update_vcpu_system_time(), which
>> is called by schedule() before context_switch(). This will
>> break the HVM guest
>> TSC_AUX state because at this point, MSR hasn't beed saved
>> for HVM guests.So
>> put the function in the point when a PV vcpu is really scheduled in.
>> 
>> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>

RE: [PATCH 01/02] pvrdtscp: move the place of write_rdtscp_aux()

[Dan Magenheimer]

I am OK with this patch [01/02] now (see other thread),
though still would like the discussion to continue
about the other patch [02/02].

Thanks,
Dan

> -----Original Message-----
> From: Xu, Dongxiao [mailto:dongxiao.xu@intel.com]
> Sent: Friday, December 11, 2009 8:31 AM
> To: Dan Magenheimer; xen-devel@lists.xensource.com
> Cc: Keir Fraser
> Subject: RE: [Xen-devel][PATCH 01/02] pvrdtscp: move the place of
> write_rdtscp_aux()
> 
> 
> Dan Magenheimer wrote:
> > This patch is wrong for TSC_MODE_PVRDTSCP.  The original
> > code should be left in __update_vcpu_system_time and
> > the context switch code should write to TSC_AUX only
> > if tsc_mode is *NOT* TSC_MODE_PVRDTSCP because the
> > implementation of full rdtscp functionality and
> > TSC_MODE_PVRDTSCP mode are incompatible.
> 
> Could you detail the wrong place? Appreciate for this!
> In my patch, I have done save/restore for guest/host 
> TSC_AUX MSR, so it will not break PVRDTSC logic. 
>  
> > 
> > But I continue to question if this should be done at all.
> > See other thread.
> > 
> >> -----Original Message-----
> >> From: Xu, Dongxiao [mailto:dongxiao.xu@intel.com]
> >> Sent: Friday, December 11, 2009 4:54 AM
> >> To: xen-devel@lists.xensource.com
> >> Cc: Keir Fraser
> >> Subject: [Xen-devel][PATCH 01/02] pvrdtscp: move the place of
> >> write_rdtscp_aux() 
> >> 
> >> 
> >>  Pvrdtscp: move write_rdtscp_aux() to paravirt_ctxt_switch_to()
> >>  - Currently write_rdtscp_aux() is placed in
> >> update_vcpu_system_time(), which
> >> is called by schedule() before context_switch(). This will
> >> break the HVM guest
> >> TSC_AUX state because at this point, MSR hasn't beed saved
> >> for HVM guests.So
> >> put the function in the point when a PV vcpu is really 
> scheduled in.
> >> 
> >> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>