Dan Magenheimer
2009-Nov-05 21:50 UTC
[Xen-devel] [PATCH] mask cpuid TSC invariant bit for various circumstances (Take 2)
Mask cpuid TSC invariant bit for some circumstances and expose it for others. On upstream Linux kernels, non-zero Invariant TSC bit permanently selects TSC as the clocksource (currently on Intel only). When these kernels run on Xen, when migration is possible and TSC is unemulated, this can cause much weirdness. But exposing non-zero Invariant TSC has performance advantages so we want to expose it when it is safe. (Note leaving it exposed/unexposed for dom0 is not an issue.) Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com> diff -r d7d7f978d704 xen/arch/x86/hvm/hvm.c --- a/xen/arch/x86/hvm/hvm.c Tue Oct 20 14:36:01 2009 +0100 +++ b/xen/arch/x86/hvm/hvm.c Thu Nov 05 11:32:25 2009 -0700 @@ -1796,6 +1796,9 @@ void hvm_cpuid(unsigned int input, unsig if ( cpuid_hypervisor_leaves(input, eax, ebx, ecx, edx) ) return; + if ( cpuid_time_leaves(input, eax, ebx, ecx, edx) ) + return; + domain_cpuid(v->domain, input, *ecx, eax, ebx, ecx, edx); switch ( input ) diff -r d7d7f978d704 xen/arch/x86/time.c --- a/xen/arch/x86/time.c Tue Oct 20 14:36:01 2009 +0100 +++ b/xen/arch/x86/time.c Thu Nov 05 11:32:25 2009 -0700 @@ -33,6 +33,7 @@ #include <asm/hpet.h> #include <io_ports.h> #include <asm/setup.h> /* for early_time_init */ +#include <public/arch-x86/cpuid.h> /* opt_clocksource: Force clocksource to one of: pit, hpet, cyclone, acpi. */ static char __initdata opt_clocksource[10]; @@ -1601,6 +1602,21 @@ void pv_soft_rdtsc(struct vcpu *v, struc regs->edx = (uint32_t)(now >> 32); } +int cpuid_time_leaves(unsigned int leaf, unsigned int *eax, unsigned int *ebx, + unsigned int *ecx, unsigned int *edx) +{ + struct domain *d = current->domain; + + if ( leaf != XEN_CPUID_APM_FUNCTION ) + return 0; + + *eax = *ebx = *ecx = 0; + *edx = cpuid_edx(leaf); + if ( !d->disable_migrate && !d->arch.vtsc ) + *edx = ~XEN_CPUID_APM_EDX_TSC_INVARIANT; + return 1; +} + /* vtsc may incur measurable performance degradation, diagnose with this */ static void dump_softtsc(unsigned char key) { diff -r d7d7f978d704 xen/arch/x86/traps.c --- a/xen/arch/x86/traps.c Tue Oct 20 14:36:01 2009 +0100 +++ b/xen/arch/x86/traps.c Thu Nov 05 11:32:25 2009 -0700 @@ -735,7 +735,8 @@ static void pv_cpuid(struct cpu_user_reg if ( current->domain->domain_id != 0 ) { - if ( !cpuid_hypervisor_leaves(a, &a, &b, &c, &d) ) + if ( !cpuid_hypervisor_leaves(a, &a, &b, &c, &d) && + !cpuid_time_leaves(a, &a, &b, &c, &d) ) domain_cpuid(current->domain, a, c, &a, &b, &c, &d); goto out; } diff -r d7d7f978d704 xen/include/asm-x86/time.h --- a/xen/include/asm-x86/time.h Tue Oct 20 14:36:01 2009 +0100 +++ b/xen/include/asm-x86/time.h Thu Nov 05 11:32:25 2009 -0700 @@ -45,4 +45,7 @@ void pv_soft_rdtsc(struct vcpu *v, struc void force_update_vcpu_system_time(struct vcpu *v); +int cpuid_time_leaves(unsigned int leaf, unsigned int *eax, unsigned int *ebx, + unsigned int *ecx, unsigned int *edx); + #endif /* __X86_TIME_H__ */ diff -r d7d7f978d704 xen/include/public/arch-x86/cpuid.h --- a/xen/include/public/arch-x86/cpuid.h Tue Oct 20 14:36:01 2009 +0100 +++ b/xen/include/public/arch-x86/cpuid.h Thu Nov 05 11:32:25 2009 -0700 @@ -65,4 +65,8 @@ #define _XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD 0 #define XEN_CPUID_FEAT1_MMU_PT_UPDATE_PRESERVE_AD (1u<<0) +/* Does the host support TSC Invariance (in Advanced Power Management)? */ +#define XEN_CPUID_APM_FUNCTION 0x80000007 +#define XEN_CPUID_APM_EDX_TSC_INVARIANT (1u<<8) + #endif /* __XEN_PUBLIC_ARCH_X86_CPUID_H__ */ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Nov-06 07:18 UTC
Re: [Xen-devel] [PATCH] mask cpuid TSC invariant bit for various circumstances (Take 2)
On 05/11/2009 21:50, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:> Mask cpuid TSC invariant bit for some circumstances > and expose it for others. On upstream Linux kernels, > non-zero Invariant TSC bit permanently selects TSC > as the clocksource (currently on Intel only). When > these kernels run on Xen, when migration is possible > and TSC is unemulated, this can cause much weirdness. > But exposing non-zero Invariant TSC has performance > advantages so we want to expose it when it is safe. > (Note leaving it exposed/unexposed for dom0 is not an > issue.)I think I pushed you into changing this in a way I like even less. :-) I can live with your original patch, so I''ll check that in after all. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2009-Nov-06 14:23 UTC
RE: [Xen-devel] [PATCH] mask cpuid TSC invariant bit for various circumstances (Take 2)
> On 05/11/2009 21:50, "Dan Magenheimer" > <dan.magenheimer@oracle.com> wrote: > > > Mask cpuid TSC invariant bit for some circumstances > > and expose it for others. On upstream Linux kernels, > > non-zero Invariant TSC bit permanently selects TSC > > as the clocksource (currently on Intel only). When > > these kernels run on Xen, when migration is possible > > and TSC is unemulated, this can cause much weirdness. > > But exposing non-zero Invariant TSC has performance > > advantages so we want to expose it when it is safe. > > (Note leaving it exposed/unexposed for dom0 is not an > > issue.) > > I think I pushed you into changing this in a way I like even > less. :-) I can > live with your original patch, so I''ll check that in after all. > > -- KeirBut note that you were correct that the original patch didn''t work with HVM domains, I presume because the xc cpuid policy code doesn''t initialize 0x80000007. That''s why I pulled the code out of the loop in domain_cpuid and then entirely out to time.c Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Nov-06 16:26 UTC
Re: [Xen-devel] [PATCH] mask cpuid TSC invariant bit for various circumstances (Take 2)
On 06/11/2009 14:23, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:>> I think I pushed you into changing this in a way I like even >> less. :-) I can >> live with your original patch, so I''ll check that in after all. >> >> -- Keir > > But note that you were correct that the original patch > didn''t work with HVM domains, I presume because the xc cpuid > policy code doesn''t initialize 0x80000007. That''s why > I pulled the code out of the loop in domain_cpuid and > then entirely out to time.cEven if it didn''t initialise 0x80000007 --- which actually I am sure it does, as DEF_MAX_EXT is defined as 0x80000008 in xc_cpuid_x86.c --- then the result will be we return all zeroes for that leaf. And that''s safe. So I think the original patch is fine for HVM guests too. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2009-Nov-09 18:16 UTC
RE: [Xen-devel] [PATCH] mask cpuid TSC invariant bit for various circumstances (Take 2)
> On 06/11/2009 14:23, "Dan Magenheimer" > <dan.magenheimer@oracle.com> wrote: > > >> I think I pushed you into changing this in a way I like even > >> less. :-) I can > >> live with your original patch, so I''ll check that in after all. > >> > >> -- Keir > > > > But note that you were correct that the original patch > > didn''t work with HVM domains, I presume because the xc cpuid > > policy code doesn''t initialize 0x80000007. That''s why > > I pulled the code out of the loop in domain_cpuid and > > then entirely out to time.c > > Even if it didn''t initialise 0x80000007 --- which actually I > am sure it > does, as DEF_MAX_EXT is defined as 0x80000008 in > xc_cpuid_x86.c --- then the > result will be we return all zeroes for that leaf. And that''s > safe. So I > think the original patch is fine for HVM guests too. > > -- KeirThought I''d wait until the final patch showed up in xen-unstable so I could test it before shooting off my mouth ;-) With the original (and your checked in) patch, it appears that the Invariant TSC bit is *always* zero for hvm. One of the points of the patch was to leave it unmasked (i.e. pass it through unchanged) under certain conditions. So... I think we need another patch now in xc_cpuid_x86. diff -r 42e268da38b9 tools/libxc/xc_cpuid_x86.c --- a/tools/libxc/xc_cpuid_x86.c Mon Nov 09 08:19:55 2009 +0000 +++ b/tools/libxc/xc_cpuid_x86.c Mon Nov 09 11:13:58 2009 -0700 @@ -237,6 +237,7 @@ static void xc_cpuid_hvm_policy( case 0x80000004: /* ... continued */ case 0x80000005: /* AMD L1 cache/TLB info (dumped by Intel policy) */ case 0x80000006: /* AMD L2/3 cache/TLB info ; Intel L2 cache features */ + case 0x80000007: /* Intel/AMD Power Management (e.g. Invariant TSC) */ break; default: _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel