On Sat, Feb 28, 2009 at 10:44:30AM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <[email protected]> wrote:
>
>>
>> True, but by how much? 212 bytes, out of 7285943 bytes which
>> is very very small percentage wise.
>
>How does this eliminate the validity of the patch?
>
It costs 212 bytes to leave is_vsmp_box() to not just be a dummy no-op.
Having is_vsmp_box() detect if the hardware is indeed vSMP, is meaningful even when
CONFIG_VSMP is not turned on. This is because is_vsmp_box() is used to
tell the kernel, that although the cpus being used are supposed to have TSCs
in sync, they are not really in sync. This is because
you cannot ensure TSCs won't drift between multiple boards being aggregated
on vSMP systems. Take the case of distro kernels. Distro kernels typically do
not have CONFIG_X86_VSMP on. Due to the large internode cacheline
setting, CONFIG_VSMP would not be on on the generic distro installer kernels.
If is_vsmp_box() is a no-op, the generic distro installer kernels will
assume TSCs to be synched, which is bad. Hence, it will be nice if, for
the cost of 212 bytes, vsmp64.o be compiled either unconditionally, OR
conditionally for 64bit architectures only. The question is, is 212 bytes out
of 7285943 bytes too expensive for the generic kernels? I hope not.
Thanks,
Kiran
Ravikiran G Thirumalai wrote:
> On Sat, Feb 28, 2009 at 10:44:30AM +0100, Ingo Molnar wrote:
>> * Ravikiran G Thirumalai <[email protected]> wrote:
>>
>>> True, but by how much? 212 bytes, out of 7285943 bytes which
>>> is very very small percentage wise.
>> How does this eliminate the validity of the patch?
>>
>
> It costs 212 bytes to leave is_vsmp_box() to not just be a dummy no-op.
> Having is_vsmp_box() detect if the hardware is indeed vSMP, is meaningful even when
> CONFIG_VSMP is not turned on. This is because is_vsmp_box() is used to
> tell the kernel, that although the cpus being used are supposed to have TSCs
> in sync, they are not really in sync. This is because
> you cannot ensure TSCs won't drift between multiple boards being aggregated
> on vSMP systems. Take the case of distro kernels. Distro kernels typically do
> not have CONFIG_X86_VSMP on. Due to the large internode cacheline
> setting, CONFIG_VSMP would not be on on the generic distro installer kernels.
> If is_vsmp_box() is a no-op, the generic distro installer kernels will
> assume TSCs to be synched, which is bad. Hence, it will be nice if, for
> the cost of 212 bytes, vsmp64.o be compiled either unconditionally, OR
> conditionally for 64bit architectures only. The question is, is 212 bytes out
> of 7285943 bytes too expensive for the generic kernels? I hope not.
we may need to revisit apic_is_clustered_box()
/*
* apic_is_clustered_box() -- Check if we can expect good TSC
*
* Thus far, the major user of this is IBM's Summit2 series:
*
* Clustered boxes may have unsynced TSC problems if they are
* multi-chassis. Use available data to take a good guess.
* If in doubt, go HPET.
*/
__cpuinit int apic_is_clustered_box(void)
....
/*
* If clusters > 2, then should be multi-chassis.
* May have to revisit this when multi-core + hyperthreaded CPUs come
* out, but AFAIK this will work even for them.
*/
with intel new cpus with 8 cores and HT enabled, even 2 sockets system will get 3 (?) so called "apic cluster"
we really need to use dmi etc way to detect multi-chassis system from now on
YH
* Ravikiran G Thirumalai <[email protected]> wrote:
> On Sat, Feb 28, 2009 at 10:44:30AM +0100, Ingo Molnar wrote:
> >
> >* Ravikiran G Thirumalai <[email protected]> wrote:
> >
> >>
> >> True, but by how much? 212 bytes, out of 7285943 bytes which
> >> is very very small percentage wise.
> >
> >How does this eliminate the validity of the patch?
> >
>
> It costs 212 bytes to leave is_vsmp_box() to not just be a dummy
> no-op. Having is_vsmp_box() detect if the hardware is indeed vSMP,
> is meaningful even when CONFIG_VSMP is not turned on. This is
> because is_vsmp_box() is used to tell the kernel, that although
> the cpus being used are supposed to have TSCs in sync, they are
> not really in sync. This is because you cannot ensure TSCs won't
> drift between multiple boards being aggregated on vSMP systems.
> Take the case of distro kernels. Distro kernels typically do not
> have CONFIG_X86_VSMP on. Due to the large internode cacheline
> setting, CONFIG_VSMP would not be on on the generic distro
> installer kernels. If is_vsmp_box() is a no-op, the generic distro
> installer kernels will assume TSCs to be synched, which is bad.
> Hence, it will be nice if, for the cost of 212 bytes, vsmp64.o be
> compiled either unconditionally, OR conditionally for 64bit
> architectures only. The question is, is 212 bytes out of 7285943
> bytes too expensive for the generic kernels? I hope not.
Sorry - got distracted and forgot about this thread. The TSC quirk
indeed looks required for your systems - you dont have a reliable
TSC due to virtualization, right?
Mind sending a patch (partial revert or so) against latest -tip that
fixes that?
Ingo
On Sun, Mar 22, 2009 at 01:48:18PM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <[email protected]> wrote:
>
>> On Sat, Feb 28, 2009 at 10:44:30AM +0100, Ingo Molnar wrote:
>> >
>> >* Ravikiran G Thirumalai <[email protected]> wrote:
>> >
>> >>
>> >> True, but by how much? 212 bytes, out of 7285943 bytes which
>> >> is very very small percentage wise.
>> >
>> >How does this eliminate the validity of the patch?
>> >
>>
>> It costs 212 bytes to leave is_vsmp_box() to not just be a dummy
>> no-op. Having is_vsmp_box() detect if the hardware is indeed vSMP,
>> is meaningful even when CONFIG_VSMP is not turned on. This is
>> because is_vsmp_box() is used to tell the kernel, that although
>> the cpus being used are supposed to have TSCs in sync, they are
>> not really in sync. This is because you cannot ensure TSCs won't
>> drift between multiple boards being aggregated on vSMP systems.
>> Take the case of distro kernels. Distro kernels typically do not
>> have CONFIG_X86_VSMP on. Due to the large internode cacheline
>> setting, CONFIG_VSMP would not be on on the generic distro
>> installer kernels. If is_vsmp_box() is a no-op, the generic distro
>> installer kernels will assume TSCs to be synched, which is bad.
>> Hence, it will be nice if, for the cost of 212 bytes, vsmp64.o be
>> compiled either unconditionally, OR conditionally for 64bit
>> architectures only. The question is, is 212 bytes out of 7285943
>> bytes too expensive for the generic kernels? I hope not.
>
>Sorry - got distracted and forgot about this thread. The TSC quirk
>indeed looks required for your systems - you dont have a reliable
>TSC due to virtualization, right?
>
Yes. Also, because there is no way to avoid tsc drift on
multiple boards/nodes.
>Mind sending a patch (partial revert or so) against latest -tip that
>fixes that?
>
Sure. Here's a revert, it is a partial revert which compiles vsmp64.c only
for 64bit architectures.
Thanks,
Kiran
Partial revert of commit 129d8bc828e011bda0b7110a097bf3a0167f966e
titled 'x86: don't compile vsmp_64 for 32bit'
Commit reverted to compile vsmp_64.c if CONFIG_X86_64 is defined,
since is_vsmp_box() needs to indicate that TSCs are not synchronized, and
hence, not a valid time source, even when CONFIG_X86_VSMP is not defined.
Signed-off-by: Ravikiran Thirumalai <[email protected]>
Index: git.tip/arch/x86/include/asm/apic.h
===================================================================
--- git.tip.orig/arch/x86/include/asm/apic.h 2009-03-23 12:50:37.000000000 -0800
+++ git.tip/arch/x86/include/asm/apic.h 2009-03-23 20:21:02.000000000 -0800
@@ -75,7 +75,7 @@ static inline void default_inquire_remot
#define setup_secondary_clock setup_secondary_APIC_clock
#endif
-#ifdef CONFIG_X86_VSMP
+#ifdef CONFIG_X86_64
extern int is_vsmp_box(void);
#else
static inline int is_vsmp_box(void)
Index: git.tip/arch/x86/include/asm/setup.h
===================================================================
--- git.tip.orig/arch/x86/include/asm/setup.h 2009-03-23 12:50:37.000000000 -0800
+++ git.tip/arch/x86/include/asm/setup.h 2009-03-23 20:19:18.000000000 -0800
@@ -64,7 +64,7 @@ extern void x86_quirk_time_init(void);
#include <asm/bootparam.h>
/* Interrupt control for vSMPowered x86_64 systems */
-#ifdef CONFIG_X86_VSMP
+#ifdef CONFIG_X86_64
void vsmp_init(void);
#else
static inline void vsmp_init(void) { }
Index: git.tip/arch/x86/kernel/Makefile
===================================================================
--- git.tip.orig/arch/x86/kernel/Makefile 2009-03-23 12:50:37.000000000 -0800
+++ git.tip/arch/x86/kernel/Makefile 2009-03-23 20:29:34.000000000 -0800
@@ -71,7 +71,6 @@ obj-$(CONFIG_FTRACE_SYSCALLS) += ftrace.
obj-$(CONFIG_KEXEC) += machine_kexec_$(BITS).o
obj-$(CONFIG_KEXEC) += relocate_kernel_$(BITS).o crash.o
obj-$(CONFIG_CRASH_DUMP) += crash_dump_$(BITS).o
-obj-$(CONFIG_X86_VSMP) += vsmp_64.o
obj-$(CONFIG_KPROBES) += kprobes.o
obj-$(CONFIG_MODULES) += module_$(BITS).o
obj-$(CONFIG_EFI) += efi.o efi_$(BITS).o efi_stub_$(BITS).o
@@ -121,4 +120,5 @@ ifeq ($(CONFIG_X86_64),y)
obj-$(CONFIG_AMD_IOMMU) += amd_iommu_init.o amd_iommu.o
obj-$(CONFIG_PCI_MMCONFIG) += mmconf-fam10h_64.o
+ obj-y += vsmp_64.o
endif
Index: git.tip/arch/x86/kernel/vsmp_64.c
===================================================================
--- git.tip.orig/arch/x86/kernel/vsmp_64.c 2009-03-23 12:50:37.000000000 -0800
+++ git.tip/arch/x86/kernel/vsmp_64.c 2009-03-23 21:09:34.000000000 -0800
@@ -22,7 +22,7 @@
#include <asm/paravirt.h>
#include <asm/setup.h>
-#ifdef CONFIG_PARAVIRT
+#if defined CONFIG_PCI && defined CONFIG_PARAVIRT
/*
* Interrupt control on vSMPowered systems:
* ~AC is a shadow of IF. If IF is 'on' AC should be 'off'
@@ -114,6 +114,7 @@ static void __init set_vsmp_pv_ops(void)
}
#endif
+#ifdef CONFIG_PCI
static int is_vsmp = -1;
static void __init detect_vsmp_box(void)
@@ -139,6 +140,15 @@ int is_vsmp_box(void)
}
}
+#else
+static void __init detect_vsmp_box(void)
+{
+}
+int is_vsmp_box(void)
+{
+ return 0;
+}
+#endif
void __init vsmp_init(void)
{
detect_vsmp_box();
* Ravikiran G Thirumalai <[email protected]> wrote:
> Sure. Here's a revert, it is a partial revert which compiles
> vsmp64.c only for 64bit architectures.
hm, why dont you add 'depends on X86_64' to the X86_VSMP Kconfig
rules? That's far clear - or am i missing some detail?
Ingo
On Tue, Mar 24, 2009 at 10:10:35AM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <[email protected]> wrote:
>
>> Sure. Here's a revert, it is a partial revert which compiles
>> vsmp64.c only for 64bit architectures.
>
>hm, why dont you add 'depends on X86_64' to the X86_VSMP Kconfig
>rules? That's far clear - or am i missing some detail?
It is already there, and has been there! But we still need to make
vsmp64.c compile only when CONFIG_X86_64 is defined no?
Thanks,
Kiran
Ravikiran G Thirumalai wrote:
> It is already there, and has been there! But we still need to make
> vsmp64.c compile only when CONFIG_X86_64 is defined no?
>
I still think you should restructure it so that vsmp_64.c only gets
compiled with CONFIG_X86_VSMP enabled. Having all that stuff compiled
into every kernel seems pretty pointless.
J
On Wed, Mar 25, 2009 at 03:16:14PM -0700, Jeremy Fitzhardinge wrote:
> Ravikiran G Thirumalai wrote:
>> It is already there, and has been there! But we still need to make
>> vsmp64.c compile only when CONFIG_X86_64 is defined no?
>>
>
> I still think you should restructure it so that vsmp_64.c only gets
> compiled with CONFIG_X86_VSMP enabled. Having all that stuff compiled into
> every kernel seems pretty pointless.
>
Well, not everything gets compiled in. Only the is_vsmp_box() logic and
related stuff gets compiled in. Other paravirt related stuff in vsmp64.c
depends on CONFIG_PARAVIRT.
Thanks,
Kiran
Ravikiran G Thirumalai wrote:
> On Wed, Mar 25, 2009 at 03:16:14PM -0700, Jeremy Fitzhardinge wrote:
>
>> Ravikiran G Thirumalai wrote:
>>
>>> It is already there, and has been there! But we still need to make
>>> vsmp64.c compile only when CONFIG_X86_64 is defined no?
>>>
>>>
>> I still think you should restructure it so that vsmp_64.c only gets
>> compiled with CONFIG_X86_VSMP enabled. Having all that stuff compiled into
>> every kernel seems pretty pointless.
>>
>>
>
> Well, not everything gets compiled in. Only the is_vsmp_box() logic and
> related stuff gets compiled in. Other paravirt related stuff in vsmp64.c
> depends on CONFIG_PARAVIRT.
>
Sure, but it would be cleaner if the whole file were controlled by
CONFIG_X86_VSMP. is_vsmp_box() is already defined as const inline
returning 0 if !CONFIG_X86_VSMP.
J
> Thanks,
> Kiran
>
On Wed, Mar 25, 2009 at 04:15:49PM -0700, Jeremy Fitzhardinge wrote:
> Ravikiran G Thirumalai wrote:
>> On Wed, Mar 25, 2009 at 03:16:14PM -0700, Jeremy Fitzhardinge wrote:
>>
>>> Ravikiran G Thirumalai wrote:
>>>
>>>> It is already there, and has been there! But we still need to make
>>>> vsmp64.c compile only when CONFIG_X86_64 is defined no?
>>>>
>>> I still think you should restructure it so that vsmp_64.c only gets
>>> compiled with CONFIG_X86_VSMP enabled. Having all that stuff compiled
>>> into every kernel seems pretty pointless.
>>>
>>>
>>
>> Well, not everything gets compiled in. Only the is_vsmp_box() logic and
>> related stuff gets compiled in. Other paravirt related stuff in vsmp64.c
>> depends on CONFIG_PARAVIRT.
>>
>
> Sure, but it would be cleaner if the whole file were controlled by
> CONFIG_X86_VSMP. is_vsmp_box() is already defined as const inline
> returning 0 if !CONFIG_X86_VSMP.
>
The point in this thread was, is_vsmp_box() needs to be meaningful even when
CONFIG_X86_VSMP is not on. This is needed because is_vsmp_box() is used to
determine if the platform has reliable tscs.
On Wed, 25 Mar 2009, Ravikiran G Thirumalai wrote:
> > Sure, but it would be cleaner if the whole file were controlled by
> > CONFIG_X86_VSMP. is_vsmp_box() is already defined as const inline
> > returning 0 if !CONFIG_X86_VSMP.
> >
>
> The point in this thread was, is_vsmp_box() needs to be meaningful even when
> CONFIG_X86_VSMP is not on. This is needed because is_vsmp_box() is used to
> determine if the platform has reliable tscs.
Did you even read what Jeremy wrote or bother to look at the code in
arch/x86/include/asm/apic.h ?
#ifdef CONFIG_X86_VSMP
extern int is_vsmp_box(void);
#else
static inline int is_vsmp_box(void)
{
return 0;
}
#endif
So the .c file is completely useless if CONFIG_X86_VSMP=n.
Thanks,
tglx
Ravikiran G Thirumalai wrote:
> The point in this thread was, is_vsmp_box() needs to be meaningful even when
> CONFIG_X86_VSMP is not on. This is needed because is_vsmp_box() is used to
> determine if the platform has reliable tscs.
>
Well, as I said, that code is inoperative at present. But aside from
that, how well will a non-VSMP kernel work on your hardware, with a
normal cacheline, etc. Is the tsc stability really all that important,
given that the kernel should notice if the tsc is busted pretty quickly
anyway.
unsynchronized_tsc() just returns a guess anyway, and if you don't have
X86_FEATURE_CONSTANT_TSC set, then it will return unstable for your
hardware anyway, even without the is_vsmp_box() test.
Failing that, you could add yourself to bad_tsc_dmi_table[] and have
that mark the tsc as unstable (you have DMI, right?).
J
On Thu, Mar 26, 2009 at 12:36:11AM +0100, Thomas Gleixner wrote:
>On Wed, 25 Mar 2009, Ravikiran G Thirumalai wrote:
>> > Sure, but it would be cleaner if the whole file were controlled by
>> > CONFIG_X86_VSMP. is_vsmp_box() is already defined as const inline
>> > returning 0 if !CONFIG_X86_VSMP.
>> >
>>
>> The point in this thread was, is_vsmp_box() needs to be meaningful even when
>> CONFIG_X86_VSMP is not on. This is needed because is_vsmp_box() is used to
>> determine if the platform has reliable tscs.
>
>Did you even read what Jeremy wrote or bother to look at the code in
>arch/x86/include/asm/apic.h ?
>
Might I politely point out the origin of this code:
http://lkml.org/lkml/2009/2/26/5
Please note the subject of the patch in the link and the subject of this
discussion. It is the same thread. The patch was commited to tip and is
still being discussed if it is any clarification :)
On Wed, Mar 25, 2009 at 04:58:59PM -0700, Jeremy Fitzhardinge wrote:
> Ravikiran G Thirumalai wrote:
>> The point in this thread was, is_vsmp_box() needs to be meaningful even
>> when
>> CONFIG_X86_VSMP is not on. This is needed because is_vsmp_box() is used
>> to
>> determine if the platform has reliable tscs.
>>
>
> Well, as I said, that code is inoperative at present. But aside from that,
If you read this thread completely and the patch that is being discussed, you'd find
that code would be operative.
Here's a threaded view of the complete discussion as we discuss for
everyone's convenience, as people seem to be jumping into the discussion
without actually reading up the context of the discussion.
http://lkml.org/lkml/2009/2/26/5
> how well will a non-VSMP kernel work on your hardware, with a normal
> cacheline, etc. Is the tsc stability really all that important, given that
> the kernel should notice if the tsc is busted pretty quickly anyway.
>
The installer kernels do not have vSMP enabled, and needs to be atleast able
to install the full distro reliably.
> unsynchronized_tsc() just returns a guess anyway, and if you don't have
> X86_FEATURE_CONSTANT_TSC set, then it will return unstable for your
> hardware anyway, even without the is_vsmp_box() test.
Unfortunately we use hardware which has X86_FEATURE_CONSTANT_TSC.
>
> Failing that, you could add yourself to bad_tsc_dmi_table[] and have that
> mark the tsc as unstable (you have DMI, right?).
>
Newer versions of the VMM does, but older ones don't :(, and obviously we
have older versions out in the field that still needs to be supported.
Commit-ID: 70511134f61bd6e5eed19f767381f9fb3e762d49
Gitweb: http://git.kernel.org/tip/70511134f61bd6e5eed19f767381f9fb3e762d49
Author: Ravikiran G Thirumalai <[email protected]>
AuthorDate: Mon, 23 Mar 2009 23:14:29 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 25 Mar 2009 21:34:28 +0100
Revert "x86: don't compile vsmp_64 for 32bit"
Partial revert of commit 129d8bc828e011bda0b7110a097bf3a0167f966e
titled 'x86: don't compile vsmp_64 for 32bit'
Commit reverted to compile vsmp_64.c if CONFIG_X86_64 is defined,
since is_vsmp_box() needs to indicate that TSCs are not synchronized, and
hence, not a valid time source, even when CONFIG_X86_VSMP is not defined.
Signed-off-by: Ravikiran Thirumalai <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
LKML-Reference: <20090324061429.GH7278@localdomain>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/apic.h | 2 +-
arch/x86/include/asm/setup.h | 2 +-
arch/x86/kernel/Makefile | 2 +-
arch/x86/kernel/vsmp_64.c | 12 +++++++++++-
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 130a9e2..df8a300 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -75,7 +75,7 @@ static inline void default_inquire_remote_apic(int apicid)
#define setup_secondary_clock setup_secondary_APIC_clock
#endif
-#ifdef CONFIG_X86_VSMP
+#ifdef CONFIG_X86_64
extern int is_vsmp_box(void);
#else
static inline int is_vsmp_box(void)
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index fbf0521..bdc2ada 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -64,7 +64,7 @@ extern void x86_quirk_time_init(void);
#include <asm/bootparam.h>
/* Interrupt control for vSMPowered x86_64 systems */
-#ifdef CONFIG_X86_VSMP
+#ifdef CONFIG_X86_64
void vsmp_init(void);
#else
static inline void vsmp_init(void) { }
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 339ce35..6e9c1f3 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -70,7 +70,6 @@ obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o
obj-$(CONFIG_KEXEC) += machine_kexec_$(BITS).o
obj-$(CONFIG_KEXEC) += relocate_kernel_$(BITS).o crash.o
obj-$(CONFIG_CRASH_DUMP) += crash_dump_$(BITS).o
-obj-$(CONFIG_X86_VSMP) += vsmp_64.o
obj-$(CONFIG_KPROBES) += kprobes.o
obj-$(CONFIG_MODULES) += module_$(BITS).o
obj-$(CONFIG_EFI) += efi.o efi_$(BITS).o efi_stub_$(BITS).o
@@ -120,4 +119,5 @@ ifeq ($(CONFIG_X86_64),y)
obj-$(CONFIG_AMD_IOMMU) += amd_iommu_init.o amd_iommu.o
obj-$(CONFIG_PCI_MMCONFIG) += mmconf-fam10h_64.o
+ obj-y += vsmp_64.o
endif
diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
index 74de562..a1d804b 100644
--- a/arch/x86/kernel/vsmp_64.c
+++ b/arch/x86/kernel/vsmp_64.c
@@ -22,7 +22,7 @@
#include <asm/paravirt.h>
#include <asm/setup.h>
-#ifdef CONFIG_PARAVIRT
+#if defined CONFIG_PCI && defined CONFIG_PARAVIRT
/*
* Interrupt control on vSMPowered systems:
* ~AC is a shadow of IF. If IF is 'on' AC should be 'off'
@@ -114,6 +114,7 @@ static void __init set_vsmp_pv_ops(void)
}
#endif
+#ifdef CONFIG_PCI
static int is_vsmp = -1;
static void __init detect_vsmp_box(void)
@@ -139,6 +140,15 @@ int is_vsmp_box(void)
}
}
+#else
+static void __init detect_vsmp_box(void)
+{
+}
+int is_vsmp_box(void)
+{
+ return 0;
+}
+#endif
void __init vsmp_init(void)
{
detect_vsmp_box();
* Ravikiran G Thirumalai <[email protected]> wrote:
> > unsynchronized_tsc() just returns a guess anyway, and if you
> > don't have X86_FEATURE_CONSTANT_TSC set, then it will return
> > unstable for your hardware anyway, even without the
> > is_vsmp_box() test.
>
> Unfortunately we use hardware which has X86_FEATURE_CONSTANT_TSC.
>
> >
> > Failing that, you could add yourself to bad_tsc_dmi_table[] and
> > have that mark the tsc as unstable (you have DMI, right?).
> >
>
> Newer versions of the VMM does, but older ones don't :(, and
> obviously we have older versions out in the field that still needs
> to be supported.
But those old versions wont have X86_FEATURE_CONSTANT_TSC set,
right?
Ingo
On Thu, Mar 26, 2009 at 10:11:53AM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <[email protected]> wrote:
>
>> > unsynchronized_tsc() just returns a guess anyway, and if you
>> > don't have X86_FEATURE_CONSTANT_TSC set, then it will return
>> > unstable for your hardware anyway, even without the
>> > is_vsmp_box() test.
>>
>> Unfortunately we use hardware which has X86_FEATURE_CONSTANT_TSC.
>>
>> >
>> > Failing that, you could add yourself to bad_tsc_dmi_table[] and
>> > have that mark the tsc as unstable (you have DMI, right?).
>> >
>>
>> Newer versions of the VMM does, but older ones don't :(, and
>> obviously we have older versions out in the field that still needs
>> to be supported.
>
>But those old versions wont have X86_FEATURE_CONSTANT_TSC set,
>right?
No, the old versions also do have X86_FEATURE_CONSTANT_TSC. The kernel
assumes that even netburst based cpus have synced tscs (of course this is
never mentioned in the intel documentation, but in the past we've
been told that that intel engineers say so -- that tscs are synced
and guaranteed to not drift between intel cpus)
Here's the code that sets X86_FEATURE_CONSTANT_TSC.
static void __cpuinit early_init_intel(struct cpuinfo_x86 *c)
{
if ((c->x86 == 0xf && c->x86_model >= 0x03) ||
(c->x86 == 0x6 && c->x86_model >= 0x0e))
set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
...
Thanks,
Kiran