Boris Ostrovsky
2012-Apr-20 02:21 UTC
[PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
# HG changeset patch
# User Boris Ostrovsky <boris.ostrovsky@amd.com>
# Date 1334875170 14400
# Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18
# Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8
svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support
TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com>
Acked-by: Wei Huang <wei.huang2@amd.com>
Tested-by: Wei Huang <wei.huang2@amd.com>
diff -r 7c777cb8f705 -r 55bf11ebce87 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c Wed Apr 18 16:49:55 2012 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c Thu Apr 19 18:39:30 2012 -0400
@@ -724,12 +724,18 @@ static void svm_set_rdtsc_exiting(struct
{
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb);
+ u32 general2_intercepts = vmcb_get_general2_intercepts(vmcb);
general1_intercepts &= ~GENERAL1_INTERCEPT_RDTSC;
- if ( enable )
+ general2_intercepts &= ~GENERAL2_INTERCEPT_RDTSCP;
+
+ if ( enable && !cpu_has_tsc_ratio ) {
general1_intercepts |= GENERAL1_INTERCEPT_RDTSC;
+ general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP;
+ }
vmcb_set_general1_intercepts(vmcb, general1_intercepts);
+ vmcb_set_general2_intercepts(vmcb, general2_intercepts);
}
static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf)
Huang2, Wei
2012-Apr-20 03:57 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
Please also backport this patch to xen-4.1. Thanks.
-Wei
-----Original Message-----
From: Boris Ostrovsky [mailto:boris.ostrovsky@amd.com]
Sent: Thursday, April 19, 2012 9:21 PM
To: JBeulich@suse.com; keir@xen.org
Cc: Ostrovsky, Boris; Huang2, Wei; xen-devel@lists.xensource.com
Subject: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by
hardware
# HG changeset patch
# User Boris Ostrovsky <boris.ostrovsky@amd.com>
# Date 1334875170 14400
# Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18
# Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8
svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support
TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com>
Acked-by: Wei Huang <wei.huang2@amd.com>
Tested-by: Wei Huang <wei.huang2@amd.com>
diff -r 7c777cb8f705 -r 55bf11ebce87 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c Wed Apr 18 16:49:55 2012 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c Thu Apr 19 18:39:30 2012 -0400
@@ -724,12 +724,18 @@ static void svm_set_rdtsc_exiting(struct
{
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb);
+ u32 general2_intercepts = vmcb_get_general2_intercepts(vmcb);
general1_intercepts &= ~GENERAL1_INTERCEPT_RDTSC;
- if ( enable )
+ general2_intercepts &= ~GENERAL2_INTERCEPT_RDTSCP;
+
+ if ( enable && !cpu_has_tsc_ratio ) {
general1_intercepts |= GENERAL1_INTERCEPT_RDTSC;
+ general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP;
+ }
vmcb_set_general1_intercepts(vmcb, general1_intercepts);
+ vmcb_set_general2_intercepts(vmcb, general2_intercepts);
}
static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf)
Keir Fraser
2012-Apr-20 08:05 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
On 20/04/2012 03:21, "Boris Ostrovsky" <boris.ostrovsky@amd.com> wrote:> # HG changeset patch > # User Boris Ostrovsky <boris.ostrovsky@amd.com> > # Date 1334875170 14400 > # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 > # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 > svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware > > When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support > TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> > Acked-by: Wei Huang <wei.huang2@amd.com> > Tested-by: Wei Huang <wei.huang2@amd.com>Worth an ack/nack from Dan M I''d say. He''ll probably have some comment about possible cross-CPU TSC skew. -- Keir> diff -r 7c777cb8f705 -r 55bf11ebce87 xen/arch/x86/hvm/svm/svm.c > --- a/xen/arch/x86/hvm/svm/svm.c Wed Apr 18 16:49:55 2012 +0100 > +++ b/xen/arch/x86/hvm/svm/svm.c Thu Apr 19 18:39:30 2012 -0400 > @@ -724,12 +724,18 @@ static void svm_set_rdtsc_exiting(struct > { > struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; > u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); > + u32 general2_intercepts = vmcb_get_general2_intercepts(vmcb); > > general1_intercepts &= ~GENERAL1_INTERCEPT_RDTSC; > - if ( enable ) > + general2_intercepts &= ~GENERAL2_INTERCEPT_RDTSCP; > + > + if ( enable && !cpu_has_tsc_ratio ) { > general1_intercepts |= GENERAL1_INTERCEPT_RDTSC; > + general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP; > + } > > vmcb_set_general1_intercepts(vmcb, general1_intercepts); > + vmcb_set_general2_intercepts(vmcb, general2_intercepts); > } > > static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf) >
Keir Fraser
2012-Apr-20 08:14 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
On 20/04/2012 09:05, "Keir Fraser" <keir.xen@gmail.com> wrote:> On 20/04/2012 03:21, "Boris Ostrovsky" <boris.ostrovsky@amd.com> wrote: > >> # HG changeset patch >> # User Boris Ostrovsky <boris.ostrovsky@amd.com> >> # Date 1334875170 14400 >> # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 >> # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 >> svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >> >> When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support >> TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. >> >> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> >> Acked-by: Wei Huang <wei.huang2@amd.com> >> Tested-by: Wei Huang <wei.huang2@amd.com> > > Worth an ack/nack from Dan M I''d say. He''ll probably have some comment about > possible cross-CPU TSC skew.Oh, and apart from that, we''re also in feature freeze for 4.2, and this isn''t a bug fix. Similarly, it''s not really a candidate for the stable 4.1 branch either, at any time. -- Keir> -- Keir > >> diff -r 7c777cb8f705 -r 55bf11ebce87 xen/arch/x86/hvm/svm/svm.c >> --- a/xen/arch/x86/hvm/svm/svm.c Wed Apr 18 16:49:55 2012 +0100 >> +++ b/xen/arch/x86/hvm/svm/svm.c Thu Apr 19 18:39:30 2012 -0400 >> @@ -724,12 +724,18 @@ static void svm_set_rdtsc_exiting(struct >> { >> struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; >> u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); >> + u32 general2_intercepts = vmcb_get_general2_intercepts(vmcb); >> >> general1_intercepts &= ~GENERAL1_INTERCEPT_RDTSC; >> - if ( enable ) >> + general2_intercepts &= ~GENERAL2_INTERCEPT_RDTSCP; >> + >> + if ( enable && !cpu_has_tsc_ratio ) { >> general1_intercepts |= GENERAL1_INTERCEPT_RDTSC; >> + general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP; >> + } >> >> vmcb_set_general1_intercepts(vmcb, general1_intercepts); >> + vmcb_set_general2_intercepts(vmcb, general2_intercepts); >> } >> >> static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf) >> > >
Jan Beulich
2012-Apr-20 08:15 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
>>> On 20.04.12 at 04:21, Boris Ostrovsky <boris.ostrovsky@amd.com> wrote: > # HG changeset patch > # User Boris Ostrovsky <boris.ostrovsky@amd.com> > # Date 1334875170 14400 > # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 > # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 > svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware > > When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support > TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions.While the patch itself looks fine, I''m having difficulty to connect the mentioning of TSC_MODE_ALWAYS_EMULATE to it - afaics all modes are affected as long as they result in d->arch.vtsc to be set. Jan> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> > Acked-by: Wei Huang <wei.huang2@amd.com> > Tested-by: Wei Huang <wei.huang2@amd.com> > > diff -r 7c777cb8f705 -r 55bf11ebce87 xen/arch/x86/hvm/svm/svm.c > --- a/xen/arch/x86/hvm/svm/svm.c Wed Apr 18 16:49:55 2012 +0100 > +++ b/xen/arch/x86/hvm/svm/svm.c Thu Apr 19 18:39:30 2012 -0400 > @@ -724,12 +724,18 @@ static void svm_set_rdtsc_exiting(struct > { > struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; > u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); > + u32 general2_intercepts = vmcb_get_general2_intercepts(vmcb); > > general1_intercepts &= ~GENERAL1_INTERCEPT_RDTSC; > - if ( enable ) > + general2_intercepts &= ~GENERAL2_INTERCEPT_RDTSCP; > + > + if ( enable && !cpu_has_tsc_ratio ) { > general1_intercepts |= GENERAL1_INTERCEPT_RDTSC; > + general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP; > + } > > vmcb_set_general1_intercepts(vmcb, general1_intercepts); > + vmcb_set_general2_intercepts(vmcb, general2_intercepts); > } > > static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf)
Keir Fraser
2012-Apr-20 08:45 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
On 20/04/2012 09:15, "Jan Beulich" <JBeulich@suse.com> wrote:>> svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >> >> When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support >> TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. > > While the patch itself looks fine, I''m having difficulty to connect the > mentioning of TSC_MODE_ALWAYS_EMULATE to it - afaics all modes > are affected as long as they result in d->arch.vtsc to be set.I think the real point of this patch is they *never* want to trap and emulate RDTSC on these newer processors. -- Keir
Dan Magenheimer
2012-Apr-20 15:02 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
> From: Keir Fraser [mailto:keir.xen@gmail.com] > Subject: Re: [Xen-devel] [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by > hardware > > On 20/04/2012 03:21, "Boris Ostrovsky" <boris.ostrovsky@amd.com> wrote: > > > # HG changeset patch > > # User Boris Ostrovsky <boris.ostrovsky@amd.com> > > # Date 1334875170 14400 > > # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 > > # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 > > svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware > > > > When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support > > TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. > > > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> > > Acked-by: Wei Huang <wei.huang2@amd.com> > > Tested-by: Wei Huang <wei.huang2@amd.com> > > Worth an ack/nack from Dan M I''d say. He''ll probably have some comment about > possible cross-CPU TSC skew.Provided the hypervisor writes the "TSC-scale-register" with the same value when any vcpu for any guest is scheduled, I don''t think there is any cross-CPU TSC skew. But my recollection is that I had a concern that, to work properly after migration, TSC scaling might need to implement: ((B + cur_tsc) * scale ) + A and AFAIK the feature only implements this for B==0. Without the rest of the implementation in the hypervisor and tools, it''s hard to determine whether my concern is valid or not. Also, I don''t recall the exact scaling process but was also concerned that a guest kernel or userland process approximating the passage of time by counting TSC cycles, they might just estimate the value once at boot (or application startup) and, due to the scaling, post-migration ticks/second might later change enough to be a problem. However, this is a generic problem with emulation as well, so the concern is really: Does the TSC scaling use the same multiplication precision as is available to emulation in the hypervisor? Dan
Huang2, Wei
2012-Apr-20 15:06 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
Hi Keir, This patch is a bug fix for 23437:d7c755c25bb9 than new feature. It slipped my hand and Boris fixed it for me. The same applies to Xen-4.1. ===== Changeset 23437 ==== HVM/SVM: enable tsc scaling ratio for SVM Future AMD CPUs support TSC scaling. It allows guests to have a different TSC frequency from host system using this formula: guest_tsc = host_tsc * tsc_ratio + vmcb_offset. The tsc_ratio is a 64bit MSR contains a fixed-point number in 8.32 format (8 bits for integer part and 32bits for fractional part). For instance 0x00000003_80000000 means tsc_ratio=3.5. This patch enables TSC scaling ratio for SVM. With it, guest VMs don''t need take #VMEXIT to calculate a translated TSC value when it is running under TSC emulation mode. This can substancially reduce the rdtsc overhead. -Wei -----Original Message----- From: Keir Fraser [mailto:keir.xen@gmail.com] On Behalf Of Keir Fraser Sent: Friday, April 20, 2012 3:15 AM To: Ostrovsky, Boris; JBeulich@suse.com; Dan Magenheimer Cc: Huang2, Wei; xen-devel@lists.xensource.com Subject: Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware On 20/04/2012 09:05, "Keir Fraser" <keir.xen@gmail.com> wrote:> On 20/04/2012 03:21, "Boris Ostrovsky" <boris.ostrovsky@amd.com> wrote: > >> # HG changeset patch >> # User Boris Ostrovsky <boris.ostrovsky@amd.com> >> # Date 1334875170 14400 >> # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 >> # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 >> svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >> >> When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support >> TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. >> >> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> >> Acked-by: Wei Huang <wei.huang2@amd.com> >> Tested-by: Wei Huang <wei.huang2@amd.com> > > Worth an ack/nack from Dan M I''d say. He''ll probably have some comment about > possible cross-CPU TSC skew.Oh, and apart from that, we''re also in feature freeze for 4.2, and this isn''t a bug fix. Similarly, it''s not really a candidate for the stable 4.1 branch either, at any time. -- Keir> -- Keir > >> diff -r 7c777cb8f705 -r 55bf11ebce87 xen/arch/x86/hvm/svm/svm.c >> --- a/xen/arch/x86/hvm/svm/svm.c Wed Apr 18 16:49:55 2012 +0100 >> +++ b/xen/arch/x86/hvm/svm/svm.c Thu Apr 19 18:39:30 2012 -0400 >> @@ -724,12 +724,18 @@ static void svm_set_rdtsc_exiting(struct >> { >> struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; >> u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); >> + u32 general2_intercepts = vmcb_get_general2_intercepts(vmcb); >> >> general1_intercepts &= ~GENERAL1_INTERCEPT_RDTSC; >> - if ( enable ) >> + general2_intercepts &= ~GENERAL2_INTERCEPT_RDTSCP; >> + >> + if ( enable && !cpu_has_tsc_ratio ) { >> general1_intercepts |= GENERAL1_INTERCEPT_RDTSC; >> + general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP; >> + } >> >> vmcb_set_general1_intercepts(vmcb, general1_intercepts); >> + vmcb_set_general2_intercepts(vmcb, general2_intercepts); >> } >> >> static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf) >> > >
Boris Ostrovsky
2012-Apr-20 15:27 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
On 04/20/12 04:45, Keir Fraser wrote:> On 20/04/2012 09:15, "Jan Beulich"<JBeulich@suse.com> wrote: > >>> svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >>> >>> When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support >>> TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. >> >> While the patch itself looks fine, I''m having difficulty to connect the >> mentioning of TSC_MODE_ALWAYS_EMULATE to it - afaics all modes >> are affected as long as they result in d->arch.vtsc to be set. > > I think the real point of this patch is they *never* want to trap and > emulate RDTSC on these newer processors.Right. Mentioning TSC_MODE_ALWAYS_EMULATE explicitly wasn''t probably a particularly good idea. The reason I did that was because with TSC_MODE_DEFAULT guests (at least Linux guests) typically don''t use TSC since TSC ends up being declared non-invariant. -boris
Wei Huang
2012-Apr-20 16:20 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
On 04/20/2012 10:02 AM, Dan Magenheimer wrote:>> From: Keir Fraser [mailto:keir.xen@gmail.com] >> Subject: Re: [Xen-devel] [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by >> hardware >> >> On 20/04/2012 03:21, "Boris Ostrovsky"<boris.ostrovsky@amd.com> wrote: >> >>> # HG changeset patch >>> # User Boris Ostrovsky<boris.ostrovsky@amd.com> >>> # Date 1334875170 14400 >>> # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 >>> # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 >>> svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >>> >>> When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support >>> TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. >>> >>> Signed-off-by: Boris Ostrovsky<boris.ostrovsky@amd.com> >>> Acked-by: Wei Huang<wei.huang2@amd.com> >>> Tested-by: Wei Huang<wei.huang2@amd.com> >> Worth an ack/nack from Dan M I''d say. He''ll probably have some comment about >> possible cross-CPU TSC skew. > Provided the hypervisor writes the "TSC-scale-register" with the same > value when any vcpu for any guest is scheduled, I don''t think > there is any cross-CPU TSC skew. > > But my recollection is that I had a concern that, to work properly > after migration, TSC scaling might need to implement: > > ((B + cur_tsc) * scale ) + AHi Dan, Thanks for your review. I tried to interpret this formula as the following: cur_tsc: Guest TSC before migration B: time elapsed (overhead) during migration scale: ratio of guest frequency/target host frequency A: target host TSC> and AFAIK the feature only implements this for B==0. > Without the rest of the implementation in the hypervisor > and tools, it''s hard to determine whether my concern is valid > or not.If my interpretation above is correct, doesn''t emulated TSC have the same problem of B == 0?> Also, I don''t recall the exact scaling process but > was also concerned that a guest kernel or userland > process approximating the passage of time by counting > TSC cycles, they might just estimate the value once at > boot (or application startup) and, due to the scaling, > post-migration ticks/second might later change enough > to be a problem. However, this > is a generic problem with emulation as well, so the > concern is really: Does the TSC scaling use the same > multiplication precision as is available to emulation > in the hypervisor?Hardware TSC scaling uses 8.32 format: 8 bits for integer part and 32 bits for fraction. Is it enough for the precision from your experience? The details of TSC scaling spec can be found on page 554 of http://support.amd.com/us/Processor_TechDocs/24593_APM_v2.pdf.> Dan > >
Keir Fraser
2012-Apr-20 16:56 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
Ah, okay. Well, depending on review feedback on list perhaps it can slip into the schedule then. :-) On 20/04/2012 16:06, "Huang2, Wei" <Wei.Huang2@amd.com> wrote:> Hi Keir, > > This patch is a bug fix for 23437:d7c755c25bb9 than new feature. It slipped my > hand and Boris fixed it for me. The same applies to Xen-4.1. > > ===== Changeset 23437 ====> > HVM/SVM: enable tsc scaling ratio for SVM > > Future AMD CPUs support TSC scaling. It allows guests to have a > different TSC frequency from host system using this formula: guest_tsc > = host_tsc * tsc_ratio + vmcb_offset. The tsc_ratio is a 64bit MSR > contains a fixed-point number in 8.32 format (8 bits for integer part > and 32bits for fractional part). For instance 0x00000003_80000000 > means tsc_ratio=3.5. > > This patch enables TSC scaling ratio for SVM. With it, guest VMs don''t > need take #VMEXIT to calculate a translated TSC value when it is > running under TSC emulation mode. This can substancially reduce the > rdtsc overhead. > > > -Wei > > -----Original Message----- > From: Keir Fraser [mailto:keir.xen@gmail.com] On Behalf Of Keir Fraser > Sent: Friday, April 20, 2012 3:15 AM > To: Ostrovsky, Boris; JBeulich@suse.com; Dan Magenheimer > Cc: Huang2, Wei; xen-devel@lists.xensource.com > Subject: Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is > supported by hardware > > On 20/04/2012 09:05, "Keir Fraser" <keir.xen@gmail.com> wrote: > >> On 20/04/2012 03:21, "Boris Ostrovsky" <boris.ostrovsky@amd.com> wrote: >> >>> # HG changeset patch >>> # User Boris Ostrovsky <boris.ostrovsky@amd.com> >>> # Date 1334875170 14400 >>> # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 >>> # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 >>> svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >>> >>> When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support >>> TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. >>> >>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> >>> Acked-by: Wei Huang <wei.huang2@amd.com> >>> Tested-by: Wei Huang <wei.huang2@amd.com> >> >> Worth an ack/nack from Dan M I''d say. He''ll probably have some comment about >> possible cross-CPU TSC skew. > > Oh, and apart from that, we''re also in feature freeze for 4.2, and this > isn''t a bug fix. Similarly, it''s not really a candidate for the stable 4.1 > branch either, at any time. > > -- Keir > >> -- Keir >> >>> diff -r 7c777cb8f705 -r 55bf11ebce87 xen/arch/x86/hvm/svm/svm.c >>> --- a/xen/arch/x86/hvm/svm/svm.c Wed Apr 18 16:49:55 2012 +0100 >>> +++ b/xen/arch/x86/hvm/svm/svm.c Thu Apr 19 18:39:30 2012 -0400 >>> @@ -724,12 +724,18 @@ static void svm_set_rdtsc_exiting(struct >>> { >>> struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; >>> u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); >>> + u32 general2_intercepts = vmcb_get_general2_intercepts(vmcb); >>> >>> general1_intercepts &= ~GENERAL1_INTERCEPT_RDTSC; >>> - if ( enable ) >>> + general2_intercepts &= ~GENERAL2_INTERCEPT_RDTSCP; >>> + >>> + if ( enable && !cpu_has_tsc_ratio ) { >>> general1_intercepts |= GENERAL1_INTERCEPT_RDTSC; >>> + general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP; >>> + } >>> >>> vmcb_set_general1_intercepts(vmcb, general1_intercepts); >>> + vmcb_set_general2_intercepts(vmcb, general2_intercepts); >>> } >>> >>> static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf) >>> >> >> > > > >
Dan Magenheimer
2012-Apr-20 17:25 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
> From: Wei Huang [mailto:wei.huang2@amd.com] > Subject: Re: [Xen-devel] [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by > hardware > > >>> > >>> When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support > >>> TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. > >>> > >>> Signed-off-by: Boris Ostrovsky<boris.ostrovsky@amd.com> > >>> Acked-by: Wei Huang<wei.huang2@amd.com> > >>> Tested-by: Wei Huang<wei.huang2@amd.com> > >> Worth an ack/nack from Dan M I''d say. He''ll probably have some comment about > >> possible cross-CPU TSC skew. > > Provided the hypervisor writes the "TSC-scale-register" with the same > > value when any vcpu for any guest is scheduled, I don''t think > > there is any cross-CPU TSC skew. > > > > But my recollection is that I had a concern that, to work properly > > after migration, TSC scaling might need to implement: > > > > ((B + cur_tsc) * scale ) + A > Hi Dan, > > Thanks for your review. I tried to interpret this formula as the following: > > cur_tsc: Guest TSC before migration > B: time elapsed (overhead) during migration > scale: ratio of guest frequency/target host frequency > A: target host TSCHi Wei -- I have to admit I don''t remember the details of the problem anymore and can''t mentally reproduce it right now. If TSC scaling is deployed (on Xen... I imagine it was designed to accommodate VMware) and works as well as emulation but faster, great! I just would hope that there is a great deal of testing done before it becomes the default because if problems do occur, they will be extremely difficult to track down.>> Without the rest of the implementation in the hypervisor >> and tools, it''s hard to determine whether my concern is valid >> or not. > If my interpretation above is correct, doesn''t emulated TSC have the > same problem of B == 0?That may be true. But if the emulation code for HVM is broken and must be fixed, that''s a much easier change than a change to a hardware instruction.> Hardware TSC scaling uses 8.32 format: 8 bits for integer part and 32 > bits for fraction. Is it enough for the precision from your experience? > The details of TSC scaling spec can be found on page 554 of > http://support.amd.com/us/Processor_TechDocs/24593_APM_v2.pdf.I didn''t write the emulation scaling code for HVM... if it uses a 32-bit scale factor, your 8+32 should be more precise. If it uses a 64-bit multiplier, it will be less precise. OTOH, the "how many TSC ticks per second" code in Xen may not be precise anyway. Keir/Jan, I still think it would be a good idea to implement a global boot-time "never trust the hardware TSC" option (default off) which would result in all guests always emulating TSC. At least this would be a "baseline" and any problems with it are due to hypervisor bugs. Dan P.S. About to be away from email for a few days.
Jan Beulich
2012-Apr-25 09:27 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
>>> On 20.04.12 at 04:21, Boris Ostrovsky <boris.ostrovsky@amd.com> wrote: > # HG changeset patch > # User Boris Ostrovsky <boris.ostrovsky@amd.com> > # Date 1334875170 14400 > # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 > # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 > svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware > > When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support > TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> > Acked-by: Wei Huang <wei.huang2@amd.com> > Tested-by: Wei Huang <wei.huang2@amd.com>So what''s the status of the discussion around this patch? Were your concerns all addressed, Dan? Is there any re-submisson necessary/planned? Jan> diff -r 7c777cb8f705 -r 55bf11ebce87 xen/arch/x86/hvm/svm/svm.c > --- a/xen/arch/x86/hvm/svm/svm.c Wed Apr 18 16:49:55 2012 +0100 > +++ b/xen/arch/x86/hvm/svm/svm.c Thu Apr 19 18:39:30 2012 -0400 > @@ -724,12 +724,18 @@ static void svm_set_rdtsc_exiting(struct > { > struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; > u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb); > + u32 general2_intercepts = vmcb_get_general2_intercepts(vmcb); > > general1_intercepts &= ~GENERAL1_INTERCEPT_RDTSC; > - if ( enable ) > + general2_intercepts &= ~GENERAL2_INTERCEPT_RDTSCP; > + > + if ( enable && !cpu_has_tsc_ratio ) { > general1_intercepts |= GENERAL1_INTERCEPT_RDTSC; > + general2_intercepts |= GENERAL2_INTERCEPT_RDTSCP; > + } > > vmcb_set_general1_intercepts(vmcb, general1_intercepts); > + vmcb_set_general2_intercepts(vmcb, general2_intercepts); > } > > static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf)
Dan Magenheimer
2012-Apr-25 15:01 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
> From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: Wednesday, April 25, 2012 3:27 AM > To: Boris Ostrovsky; Dan Magenheimer > Cc: wei.huang2@amd.com; xen-devel; keir@xen.org > Subject: Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware > > >>> On 20.04.12 at 04:21, Boris Ostrovsky <boris.ostrovsky@amd.com> wrote: > > # HG changeset patch > > # User Boris Ostrovsky <boris.ostrovsky@amd.com> > > # Date 1334875170 14400 > > # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 > > # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 > > svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware > > > > When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support > > TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. > > > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> > > Acked-by: Wei Huang <wei.huang2@amd.com> > > Tested-by: Wei Huang <wei.huang2@amd.com> > > So what''s the status of the discussion around this patch? Were > your concerns all addressed, Dan? Is there any re-submisson > necessary/planned?My concerns will be addressed when there is a fully-functional adequately-tested full-stack implementation, rather than "we have a new instruction that should solve (part of) this problem, let''s turn it on by default." While I wish I could invest the time required to do (or participate in) the testing, sadly I can''t, so I understand if my opinion is discarded.
Jan Beulich
2012-Apr-25 15:51 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
>>> On 25.04.12 at 17:01, Dan Magenheimer <dan.magenheimer@oracle.com> wrote: >> From: Jan Beulich [mailto:JBeulich@suse.com] >> Sent: Wednesday, April 25, 2012 3:27 AM >> To: Boris Ostrovsky; Dan Magenheimer >> Cc: wei.huang2@amd.com; xen-devel; keir@xen.org >> Subject: Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is > supported by hardware >> >> >>> On 20.04.12 at 04:21, Boris Ostrovsky <boris.ostrovsky@amd.com> wrote: >> > # HG changeset patch >> > # User Boris Ostrovsky <boris.ostrovsky@amd.com> >> > # Date 1334875170 14400 >> > # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 >> > # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 >> > svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >> > >> > When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support >> > TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. >> > >> > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> >> > Acked-by: Wei Huang <wei.huang2@amd.com> >> > Tested-by: Wei Huang <wei.huang2@amd.com> >> >> So what''s the status of the discussion around this patch? Were >> your concerns all addressed, Dan? Is there any re-submisson >> necessary/planned? > > My concerns will be addressed when there is a fully-functional > adequately-tested full-stack implementation, rather than "we > have a new instruction that should solve (part of) this problem, > let''s turn it on by default." > > While I wish I could invest the time required to do (or > participate in) the testing, sadly I can''t, so I understand > if my opinion is discarded.As Keir had asked to get an ACK/NAK from you - is this then a NAK or a "don''t care" or yet something else (it doesn''t read anywhere close to an ACK in any case). Jan
Wei Huang
2012-Apr-25 16:04 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
On 04/25/2012 11:05 AM, Dan Magenheimer wrote:>> From: Jan Beulich [mailto:JBeulich@suse.com] >> Subject: RE: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >> >>>>> On 25.04.12 at 17:01, Dan Magenheimer<dan.magenheimer@oracle.com> wrote: >>>> From: Jan Beulich [mailto:JBeulich@suse.com] >>>> Sent: Wednesday, April 25, 2012 3:27 AM >>>> To: Boris Ostrovsky; Dan Magenheimer >>>> Cc: wei.huang2@amd.com; xen-devel; keir@xen.org >>>> Subject: Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is >>> supported by hardware >>>>>>> On 20.04.12 at 04:21, Boris Ostrovsky<boris.ostrovsky@amd.com> wrote: >>>>> # HG changeset patch >>>>> # User Boris Ostrovsky<boris.ostrovsky@amd.com> >>>>> # Date 1334875170 14400 >>>>> # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 >>>>> # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 >>>>> svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware >>>>> >>>>> When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support >>>>> TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. >>>>> >>>>> Signed-off-by: Boris Ostrovsky<boris.ostrovsky@amd.com> >>>>> Acked-by: Wei Huang<wei.huang2@amd.com> >>>>> Tested-by: Wei Huang<wei.huang2@amd.com> >>>> So what''s the status of the discussion around this patch? Were >>>> your concerns all addressed, Dan? Is there any re-submisson >>>> necessary/planned? >>> My concerns will be addressed when there is a fully-functional >>> adequately-tested full-stack implementation, rather than "we >>> have a new instruction that should solve (part of) this problem, >>> let''s turn it on by default." >>> >>> While I wish I could invest the time required to do (or >>> participate in) the testing, sadly I can''t, so I understand >>> if my opinion is discarded. >> As Keir had asked to get an ACK/NAK from you - is this then a NAK >> or a "don''t care" or yet something else (it doesn''t read anywhere >> close to an ACK in any case). > Something else ;-) > > I certainly don''t feel comfortable ACKing it. I''d like > to see some testing that demonstrates the patch either improves > functionality or performance without breaking other things. > But if nobody else shares my concern, I don''t feel that > I have the right to block it either.We can provide some rdtsc performance numbers. Regarding functionality, it is relatively hard to prove unless Dan has some more specific ideas of testing it. I think the hardware rdtsc scaling is inline with software-based emulated approach. -Wei>
Dan Magenheimer
2012-Apr-25 16:05 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
> From: Jan Beulich [mailto:JBeulich@suse.com] > Subject: RE: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware > > >>> On 25.04.12 at 17:01, Dan Magenheimer <dan.magenheimer@oracle.com> wrote: > >> From: Jan Beulich [mailto:JBeulich@suse.com] > >> Sent: Wednesday, April 25, 2012 3:27 AM > >> To: Boris Ostrovsky; Dan Magenheimer > >> Cc: wei.huang2@amd.com; xen-devel; keir@xen.org > >> Subject: Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is > > supported by hardware > >> > >> >>> On 20.04.12 at 04:21, Boris Ostrovsky <boris.ostrovsky@amd.com> wrote: > >> > # HG changeset patch > >> > # User Boris Ostrovsky <boris.ostrovsky@amd.com> > >> > # Date 1334875170 14400 > >> > # Node ID 55bf11ebce87ceb73fb2c372dcef170ec0bb4a18 > >> > # Parent 7c777cb8f705411b77c551f34ba88bdc09e38ab8 > >> > svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware > >> > > >> > When running in TSC_MODE_ALWAYS_EMULATE mode on processors that support > >> > TSC scaling we don''t need to intercept RDTSC/RDTSCP instructions. > >> > > >> > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> > >> > Acked-by: Wei Huang <wei.huang2@amd.com> > >> > Tested-by: Wei Huang <wei.huang2@amd.com> > >> > >> So what''s the status of the discussion around this patch? Were > >> your concerns all addressed, Dan? Is there any re-submisson > >> necessary/planned? > > > > My concerns will be addressed when there is a fully-functional > > adequately-tested full-stack implementation, rather than "we > > have a new instruction that should solve (part of) this problem, > > let''s turn it on by default." > > > > While I wish I could invest the time required to do (or > > participate in) the testing, sadly I can''t, so I understand > > if my opinion is discarded. > > As Keir had asked to get an ACK/NAK from you - is this then a NAK > or a "don''t care" or yet something else (it doesn''t read anywhere > close to an ACK in any case).Something else ;-) I certainly don''t feel comfortable ACKing it. I''d like to see some testing that demonstrates the patch either improves functionality or performance without breaking other things. But if nobody else shares my concern, I don''t feel that I have the right to block it either.
Keir Fraser
2012-Apr-25 17:14 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
On 25/04/2012 17:04, "Wei Huang" <wei.huang2@amd.com> wrote:>> I certainly don''t feel comfortable ACKing it. I''d like >> to see some testing that demonstrates the patch either improves >> functionality or performance without breaking other things. >> But if nobody else shares my concern, I don''t feel that >> I have the right to block it either. > > We can provide some rdtsc performance numbers. Regarding functionality, > it is relatively hard to prove unless Dan has some more specific ideas > of testing it. I think the hardware rdtsc scaling is inline with > software-based emulated approach.I''ve put it in to xen-unstable. I''m not sure about putting into 4.1 for the forthcoming release from that branch. -- Keir
Boris Ostrovsky
2012-Apr-25 20:07 UTC
Re: [PATCH] svm: Do not intercept RDTCS(P) when TSC scaling is supported by hardware
On 04/25/2012 01:14 PM, Keir Fraser wrote:> On 25/04/2012 17:04, "Wei Huang"<wei.huang2@amd.com> wrote: > >>> I certainly don''t feel comfortable ACKing it. I''d like >>> to see some testing that demonstrates the patch either improves >>> functionality or performance without breaking other things. >>> But if nobody else shares my concern, I don''t feel that >>> I have the right to block it either. >> >> We can provide some rdtsc performance numbers. Regarding functionality, >> it is relatively hard to prove unless Dan has some more specific ideas >> of testing it. I think the hardware rdtsc scaling is inline with >> software-based emulated approach. > > I''ve put it in to xen-unstable. I''m not sure about putting into 4.1 for the > forthcoming release from that branch.As far as performance numbers are concerned, with this patch applied (i.e. native execution) RDTSC instruction executed in a loop takes about 150ns per iteration. That''s including a few instructions for loop control. With full SW emulation (pre-patch) the same loop is executed in roughly 6us. Obviously this is not a realistic scenario but it gives a feel of what the patch provides. -boris