[PATCH] x86: don't compile vsmp_64 for 32bit

[PATCH] x86: don't compile vsmp_64 for 32bit

[Yinghai Lu]

Impact: cleanup

that is only needed when CONFIG_X86_VSMP is defined with 64bit
also remove dead code about PCI, because CONFIG_X86_VSMP depends on PCI

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/include/asm/apic.h  |    7 +++++++
 arch/x86/include/asm/setup.h |    4 ++++
 arch/x86/kernel/Makefile     |    2 +-
 arch/x86/kernel/setup.c      |    2 --
 arch/x86/kernel/vsmp_64.c    |   12 +-----------
 5 files changed, 13 insertions(+), 14 deletions(-)

Index: linux-2.6/arch/x86/include/asm/apic.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/apic.h
+++ linux-2.6/arch/x86/include/asm/apic.h
@@ -75,7 +75,14 @@ static inline void default_inquire_remot
 #define setup_secondary_clock setup_secondary_APIC_clock
 #endif
 
+#ifdef CONFIG_X86_VSMP
 extern int is_vsmp_box(void);
+#else
+static inline int is_vsmp_box(void)
+{
+	return 0;
+}
+#endif
 extern void xapic_wait_icr_idle(void);
 extern u32 safe_xapic_wait_icr_idle(void);
 extern void xapic_icr_write(u32, u32);
Index: linux-2.6/arch/x86/include/asm/setup.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/setup.h
+++ linux-2.6/arch/x86/include/asm/setup.h
@@ -64,7 +64,11 @@ extern void x86_quirk_time_init(void);
 #include <asm/bootparam.h>
 
 /* Interrupt control for vSMPowered x86_64 systems */
+#ifdef CONFIG_X86_VSMP
 void vsmp_init(void);
+#else
+static inline void vsmp_init(void) { }
+#endif
 
 void setup_bios_corruption_check(void);
 
Index: linux-2.6/arch/x86/kernel/Makefile
===================================================================
--- linux-2.6.orig/arch/x86/kernel/Makefile
+++ linux-2.6/arch/x86/kernel/Makefile
@@ -70,7 +70,7 @@ obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= f
 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-y				+= vsmp_64.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
Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c
@@ -863,9 +863,7 @@ void __init setup_arch(char **cmdline_p)
 
 	reserve_initrd();
 
-#ifdef CONFIG_X86_64
 	vsmp_init();
-#endif
 
 	io_delay_init();
 
Index: linux-2.6/arch/x86/kernel/vsmp_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/vsmp_64.c
+++ linux-2.6/arch/x86/kernel/vsmp_64.c
@@ -22,7 +22,7 @@
 #include <asm/paravirt.h>
 #include <asm/setup.h>
 
-#if defined CONFIG_PCI && defined CONFIG_PARAVIRT
+#ifdef CONFIG_PARAVIRT
 /*
  * Interrupt control on vSMPowered systems:
  * ~AC is a shadow of IF.  If IF is 'on' AC should be 'off'
@@ -114,7 +114,6 @@ 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,15 +138,6 @@ int is_vsmp_box(void)
 		return 0;
 	}
 }
-#else
-static void __init detect_vsmp_box(void)
-{
-}
-int is_vsmp_box(void)
-{
-	return 0;
-}
-#endif
 
 void __init vsmp_init(void)
 {

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> 
> Impact: cleanup
> 
> that is only needed when CONFIG_X86_VSMP is defined with 64bit
> also remove dead code about PCI, because CONFIG_X86_VSMP depends on PCI
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  arch/x86/include/asm/apic.h  |    7 +++++++
>  arch/x86/include/asm/setup.h |    4 ++++
>  arch/x86/kernel/Makefile     |    2 +-
>  arch/x86/kernel/setup.c      |    2 --
>  arch/x86/kernel/vsmp_64.c    |   12 +-----------
>  5 files changed, 13 insertions(+), 14 deletions(-)

Applied to tip:x86/apic, thanks Yinghai!

	Ingo

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

On Wed, Feb 25, 2009 at 09:20:50PM -0800, Yinghai Lu wrote:
>
>Impact: cleanup
>
>that is only needed when CONFIG_X86_VSMP is defined with 64bit
>also remove dead code about PCI, because CONFIG_X86_VSMP depends on PCI
>
>Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>

NAK!
vsmp64.c is compiled unconditionally for a reason.  There are ifdefs in the
file to avoid code compilation based on config options. is_vsmp_box() is
needed even when CONFIG_X86_VSMP is not enabled, since distro kernels don't ship
with CONFIG_X86_VSMP, and since is_vsmp_box() is used to determine  whether
tsc's can be considered synced or not, this is needed.

In the future, I would appreciate if you copy me on cleanups/changes involving
vsmp64.c

Thanks,
Kiran

>---
> arch/x86/include/asm/apic.h  |    7 +++++++
> arch/x86/include/asm/setup.h |    4 ++++
> arch/x86/kernel/Makefile     |    2 +-
> arch/x86/kernel/setup.c      |    2 --
> arch/x86/kernel/vsmp_64.c    |   12 +-----------
> 5 files changed, 13 insertions(+), 14 deletions(-)
>
>Index: linux-2.6/arch/x86/include/asm/apic.h
>===================================================================
>--- linux-2.6.orig/arch/x86/include/asm/apic.h
>+++ linux-2.6/arch/x86/include/asm/apic.h
>@@ -75,7 +75,14 @@ static inline void default_inquire_remot
> #define setup_secondary_clock setup_secondary_APIC_clock
> #endif
> 
>+#ifdef CONFIG_X86_VSMP
> extern int is_vsmp_box(void);
>+#else
>+static inline int is_vsmp_box(void)
>+{
>+	return 0;
>+}
>+#endif
> extern void xapic_wait_icr_idle(void);
> extern u32 safe_xapic_wait_icr_idle(void);
> extern void xapic_icr_write(u32, u32);
>Index: linux-2.6/arch/x86/include/asm/setup.h
>===================================================================
>--- linux-2.6.orig/arch/x86/include/asm/setup.h
>+++ linux-2.6/arch/x86/include/asm/setup.h
>@@ -64,7 +64,11 @@ extern void x86_quirk_time_init(void);
> #include <asm/bootparam.h>
> 
> /* Interrupt control for vSMPowered x86_64 systems */
>+#ifdef CONFIG_X86_VSMP
> void vsmp_init(void);
>+#else
>+static inline void vsmp_init(void) { }
>+#endif
> 
> void setup_bios_corruption_check(void);
> 
>Index: linux-2.6/arch/x86/kernel/Makefile
>===================================================================
>--- linux-2.6.orig/arch/x86/kernel/Makefile
>+++ linux-2.6/arch/x86/kernel/Makefile
>@@ -70,7 +70,7 @@ obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= f
> 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-y				+= vsmp_64.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
>Index: linux-2.6/arch/x86/kernel/setup.c
>===================================================================
>--- linux-2.6.orig/arch/x86/kernel/setup.c
>+++ linux-2.6/arch/x86/kernel/setup.c
>@@ -863,9 +863,7 @@ void __init setup_arch(char **cmdline_p)
> 
> 	reserve_initrd();
> 
>-#ifdef CONFIG_X86_64
> 	vsmp_init();
>-#endif
> 
> 	io_delay_init();
> 
>Index: linux-2.6/arch/x86/kernel/vsmp_64.c
>===================================================================
>--- linux-2.6.orig/arch/x86/kernel/vsmp_64.c
>+++ linux-2.6/arch/x86/kernel/vsmp_64.c
>@@ -22,7 +22,7 @@
> #include <asm/paravirt.h>
> #include <asm/setup.h>
> 
>-#if defined CONFIG_PCI && defined CONFIG_PARAVIRT
>+#ifdef CONFIG_PARAVIRT
> /*
>  * Interrupt control on vSMPowered systems:
>  * ~AC is a shadow of IF.  If IF is 'on' AC should be 'off'
>@@ -114,7 +114,6 @@ 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,15 +138,6 @@ int is_vsmp_box(void)
> 		return 0;
> 	}
> }
>-#else
>-static void __init detect_vsmp_box(void)
>-{
>-}
>-int is_vsmp_box(void)
>-{
>-	return 0;
>-}
>-#endif
> 
> void __init vsmp_init(void)
> {
>--
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Yinghai Lu]

Ravikiran G Thirumalai wrote:
> On Wed, Feb 25, 2009 at 09:20:50PM -0800, Yinghai Lu wrote:
>> Impact: cleanup
>>
>> that is only needed when CONFIG_X86_VSMP is defined with 64bit
>> also remove dead code about PCI, because CONFIG_X86_VSMP depends on PCI
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
> 
> NAK!
> vsmp64.c is compiled unconditionally for a reason.  There are ifdefs in the
> file to avoid code compilation based on config options. is_vsmp_box() is
> needed even when CONFIG_X86_VSMP is not enabled, since distro kernels don't ship
> with CONFIG_X86_VSMP, and since is_vsmp_box() is used to determine  whether
> tsc's can be considered synced or not, this is needed.

so even CONFIG_X86_VSMP is not defined, you want all x86 systems including 32bit kernel and 64bit kernel 
to call

static int is_vsmp = -1;

static void __init detect_vsmp_box(void)
{
        is_vsmp = 0;

        if (!early_pci_allowed())
                return;

        /* Check if we are running on a ScaleMP vSMPowered box */
        if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
             (PCI_VENDOR_ID_SCALEMP | (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
                is_vsmp = 1;
}

int is_vsmp_box(void)
{
        if (is_vsmp != -1)
                return is_vsmp;
        else {
                WARN_ON_ONCE(1);
                return 0;
        }
}


> 
> In the future, I would appreciate if you copy me on cleanups/changes involving
> vsmp64.c

i didn't touch vsmp64.c

YH

> 
> Thanks,
> Kiran
> 
>> ---
>> arch/x86/include/asm/apic.h  |    7 +++++++
>> arch/x86/include/asm/setup.h |    4 ++++
>> arch/x86/kernel/Makefile     |    2 +-
>> arch/x86/kernel/setup.c      |    2 --
>> arch/x86/kernel/vsmp_64.c    |   12 +-----------
>> 5 files changed, 13 insertions(+), 14 deletions(-)
>>
>> Index: linux-2.6/arch/x86/include/asm/apic.h
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/include/asm/apic.h
>> +++ linux-2.6/arch/x86/include/asm/apic.h
>> @@ -75,7 +75,14 @@ static inline void default_inquire_remot
>> #define setup_secondary_clock setup_secondary_APIC_clock
>> #endif
>>
>> +#ifdef CONFIG_X86_VSMP
>> extern int is_vsmp_box(void);
>> +#else
>> +static inline int is_vsmp_box(void)
>> +{
>> +	return 0;
>> +}
>> +#endif
>> extern void xapic_wait_icr_idle(void);
>> extern u32 safe_xapic_wait_icr_idle(void);
>> extern void xapic_icr_write(u32, u32);
>> Index: linux-2.6/arch/x86/include/asm/setup.h
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/include/asm/setup.h
>> +++ linux-2.6/arch/x86/include/asm/setup.h
>> @@ -64,7 +64,11 @@ extern void x86_quirk_time_init(void);
>> #include <asm/bootparam.h>
>>
>> /* Interrupt control for vSMPowered x86_64 systems */
>> +#ifdef CONFIG_X86_VSMP
>> void vsmp_init(void);
>> +#else
>> +static inline void vsmp_init(void) { }
>> +#endif
>>
>> void setup_bios_corruption_check(void);
>>
>> Index: linux-2.6/arch/x86/kernel/Makefile
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/Makefile
>> +++ linux-2.6/arch/x86/kernel/Makefile
>> @@ -70,7 +70,7 @@ obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= f
>> 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-y				+= vsmp_64.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
>> Index: linux-2.6/arch/x86/kernel/setup.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/setup.c
>> +++ linux-2.6/arch/x86/kernel/setup.c
>> @@ -863,9 +863,7 @@ void __init setup_arch(char **cmdline_p)
>>
>> 	reserve_initrd();
>>
>> -#ifdef CONFIG_X86_64
>> 	vsmp_init();
>> -#endif
>>
>> 	io_delay_init();
>>
>> Index: linux-2.6/arch/x86/kernel/vsmp_64.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/vsmp_64.c
>> +++ linux-2.6/arch/x86/kernel/vsmp_64.c
>> @@ -22,7 +22,7 @@
>> #include <asm/paravirt.h>
>> #include <asm/setup.h>
>>
>> -#if defined CONFIG_PCI && defined CONFIG_PARAVIRT
>> +#ifdef CONFIG_PARAVIRT
>> /*
>>  * Interrupt control on vSMPowered systems:
>>  * ~AC is a shadow of IF.  If IF is 'on' AC should be 'off'
>> @@ -114,7 +114,6 @@ 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,15 +138,6 @@ int is_vsmp_box(void)
>> 		return 0;
>> 	}
>> }
>> -#else
>> -static void __init detect_vsmp_box(void)
>> -{
>> -}
>> -int is_vsmp_box(void)
>> -{
>> -	return 0;
>> -}
>> -#endif
>>
>> void __init vsmp_init(void)
>> {
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ingo Molnar]

* Ravikiran G Thirumalai <kiran@scalex86.org> wrote:

> On Wed, Feb 25, 2009 at 09:20:50PM -0800, Yinghai Lu wrote:
> >
> >Impact: cleanup
> >
> >that is only needed when CONFIG_X86_VSMP is defined with 64bit
> >also remove dead code about PCI, because CONFIG_X86_VSMP depends on PCI
> >
> >Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >
> 
> NAK!
> vsmp64.c is compiled unconditionally for a reason.  There are ifdefs in the
> file to avoid code compilation based on config options. is_vsmp_box() is
> needed even when CONFIG_X86_VSMP is not enabled, since distro kernels don't ship
> with CONFIG_X86_VSMP, and since is_vsmp_box() is used to determine  whether
> tsc's can be considered synced or not, this is needed.

is_vsmp_box() is always available:

> >+#ifdef CONFIG_X86_VSMP
> > extern int is_vsmp_box(void);
> >+#else
> >+static inline int is_vsmp_box(void)
> >+{
> >+	return 0;
> >+}
> >+#endif

What this patch does is it reduces the kernel's size when 
CONFIG_X86_VSMP is turned off and also makes the code arguably 
cleaner.

> In the future, I would appreciate if you copy me on 
> cleanups/changes involving vsmp64.c

That is what i did when i queued up the change. That is how you 
became aware of this commit.

	Ingo

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

On Thu, Feb 26, 2009 at 12:44:57PM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
>
>> On Wed, Feb 25, 2009 at 09:20:50PM -0800, Yinghai Lu wrote:
>> >
>> >Impact: cleanup
>> >
>> >that is only needed when CONFIG_X86_VSMP is defined with 64bit
>> >also remove dead code about PCI, because CONFIG_X86_VSMP depends on PCI
>> >
>> >Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> >
>> 
>> NAK!
>> vsmp64.c is compiled unconditionally for a reason.  There are ifdefs in the
>> file to avoid code compilation based on config options. is_vsmp_box() is
>> needed even when CONFIG_X86_VSMP is not enabled, since distro kernels don't ship
>> with CONFIG_X86_VSMP, and since is_vsmp_box() is used to determine  whether
>> tsc's can be considered synced or not, this is needed.
>
>is_vsmp_box() is always available:

Yes.  But it does not really detect if the machine is vsmp.  It is just a
'return 0'.

>
>> >+#ifdef CONFIG_X86_VSMP
>> > extern int is_vsmp_box(void);
>> >+#else
>> >+static inline int is_vsmp_box(void)
>> >+{
>> >+	return 0;
>> >+}
>> >+#endif
>
>What this patch does is it reduces the kernel's size when 
>CONFIG_X86_VSMP is turned off and also makes the code arguably 
>cleaner.

True, but by how much? 212 bytes, out of 7285943 bytes which is very
very small percentage wise.

-bash-3.1$ size results/2.6.29-rc6-defconfig/vmlinux
   text    data     bss     dec     hex filename
7285943 1482684  812968 9581595  92341b results/2.6.29-rc6-defconfig/vmlinux
-bash-3.1$ size results/2.6.29-rc6-defconfig+patch/vmlinux
   text    data     bss     dec     hex filename
7285731 1482684  812968 9581383  923347 results/2.6.29-rc6-defconfig+patch/vmlinux

Note that most of the code within vsmp64.c is dependent on PARAVIRT and
won't get compiled if PARAVIRT is not enabled.

We made this code dependent on PARAVIRT rather than CONFIG_X86_VSMP because,
vsmp support has two aspects to it.  One is a minimal paravirtualization part
and the other is internode cacheline support.  While CONFIG_X86_VSMP enables
both, all that is needed for the paravirtualization part is PARAVIRT.  And
it is not heavy or penalizing like the internode cacheline setting.   This will
mean installation of distros with the distro provided installer kernels can be done
relatively easily if paravirt is enabled.   That's all!

>
>> In the future, I would appreciate if you copy me on 
>> cleanups/changes involving vsmp64.c
>
>That is what i did when i queued up the change. That is how you 
>became aware of this commit.
>

Thanks for that.  But I started replying to the message before
the patch was queued/received your email.  But the patch was
queued by the time i finished replying.

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ingo Molnar]

* Ravikiran G Thirumalai <kiran@scalex86.org> wrote:

> On Thu, Feb 26, 2009 at 12:44:57PM +0100, Ingo Molnar wrote:
> >
> >* Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
> >
> >> On Wed, Feb 25, 2009 at 09:20:50PM -0800, Yinghai Lu wrote:
> >> >
> >> >Impact: cleanup
> >> >
> >> >that is only needed when CONFIG_X86_VSMP is defined with 64bit
> >> >also remove dead code about PCI, because CONFIG_X86_VSMP depends on PCI
> >> >
> >> >Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >> >
> >> 
> >> NAK!
> >> vsmp64.c is compiled unconditionally for a reason.  There are ifdefs in the
> >> file to avoid code compilation based on config options. is_vsmp_box() is
> >> needed even when CONFIG_X86_VSMP is not enabled, since distro kernels don't ship
> >> with CONFIG_X86_VSMP, and since is_vsmp_box() is used to determine  whether
> >> tsc's can be considered synced or not, this is needed.
> >
> >is_vsmp_box() is always available:
> 
> Yes.  But it does not really detect if the machine is vsmp.  
> It is just a 'return 0'.
> 
> >
> >> >+#ifdef CONFIG_X86_VSMP
> >> > extern int is_vsmp_box(void);
> >> >+#else
> >> >+static inline int is_vsmp_box(void)
> >> >+{
> >> >+	return 0;
> >> >+}
> >> >+#endif
> >
> >What this patch does is it reduces the kernel's size when 
> >CONFIG_X86_VSMP is turned off and also makes the code arguably 
> >cleaner.
> 
> 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?

	Ingo

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

On Sat, Feb 28, 2009 at 10:44:30AM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <kiran@scalex86.org> 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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Yinghai Lu]

Ravikiran G Thirumalai wrote:
> On Sat, Feb 28, 2009 at 10:44:30AM +0100, Ingo Molnar wrote:
>> * Ravikiran G Thirumalai <kiran@scalex86.org> 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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ingo Molnar]

* Ravikiran G Thirumalai <kiran@scalex86.org> wrote:

> On Sat, Feb 28, 2009 at 10:44:30AM +0100, Ingo Molnar wrote:
> >
> >* Ravikiran G Thirumalai <kiran@scalex86.org> 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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

On Sun, Mar 22, 2009 at 01:48:18PM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
>
>> On Sat, Feb 28, 2009 at 10:44:30AM +0100, Ingo Molnar wrote:
>> >
>> >* Ravikiran G Thirumalai <kiran@scalex86.org> 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 <kiran@scalex86.org>

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();

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ingo Molnar]

* Ravikiran G Thirumalai <kiran@scalex86.org> 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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

On Tue, Mar 24, 2009 at 10:10:35AM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <kiran@scalex86.org> 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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Jeremy Fitzhardinge]

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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Jeremy Fitzhardinge]

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
>   

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

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.

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Thomas Gleixner]

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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Jeremy Fitzhardinge]

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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

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 :)

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

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.

[tip:x86/apic] Revert "x86: don't compile vsmp_64 for 32bit"

[Ravikiran G Thirumalai]

Commit-ID:  70511134f61bd6e5eed19f767381f9fb3e762d49
Gitweb:     http://git.kernel.org/tip/70511134f61bd6e5eed19f767381f9fb3e762d49
Author:     Ravikiran G Thirumalai <kiran@scalex86.org>
AuthorDate: Mon, 23 Mar 2009 23:14:29 -0700
Committer:  Ingo Molnar <mingo@elte.hu>
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 <kiran@scalex86.org>
Cc: Yinghai Lu <yinghai@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: shai@scalex86.org
LKML-Reference: <20090324061429.GH7278@localdomain>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 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();

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ingo Molnar]

* Ravikiran G Thirumalai <kiran@scalex86.org> 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

Re: [PATCH] x86: don't compile vsmp_64 for 32bit

[Ravikiran G Thirumalai]

On Thu, Mar 26, 2009 at 10:11:53AM +0100, Ingo Molnar wrote:
>
>* Ravikiran G Thirumalai <kiran@scalex86.org> 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