Re: + x86-vsmp-build-fixes.patch added to -mm tree

Re: + x86-vsmp-build-fixes.patch added to -mm tree

[Yinghai Lu]

On Mon, Mar 17, 2008 at 10:14 PM,  <akpm@linux-foundation.org> wrote:
>
>  The patch titled
>      x86: vsmp build fixes
>  has been added to the -mm tree.  Its filename is
>      x86-vsmp-build-fixes.patch
>
>  Before you just go and hit "reply", please:
>    a) Consider who else should be cc'ed
>    b) Prefer to cc a suitable mailing list as well
>    c) Ideally: find the original patch on the mailing list and do a
>       reply-to-all to that, adding suitable additional cc's
>
>  *** Remember to use Documentation/SubmitChecklist when testing your code ***
>
>  See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
>  out what to do about this
>
>  The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>
>  ------------------------------------------------------
>  Subject: x86: vsmp build fixes
>  From: Glauber Costa <gcosta@redhat.com>
>
>  VSMP depends on PCI, but the file is now compiled conditionally on PARAVIRT,
>  no VSMP, so enclose everything in an ifdef CONFIG_PCI.
>
>  We have to be careful enough to let is_vsmp_box and vsmp_init defined, since
>  they are used outside this file
>
>  Signed-off-by: Glauber Costa <gcosta@redhat.com>
>  Cc: Ravikiran Thirumalai <kiran@scalemp.com>
>  Cc: Ingo Molnar <mingo@elte.hu>
>  Cc: Thomas Gleixner <tglx@linutronix.de>
>  Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>  ---
>
>   arch/x86/kernel/vsmp_64.c |   10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
>  diff -puN arch/x86/kernel/vsmp_64.c~x86-vsmp-build-fixes arch/x86/kernel/vsmp_64.c
>  --- a/arch/x86/kernel/vsmp_64.c~x86-vsmp-build-fixes
>  +++ a/arch/x86/kernel/vsmp_64.c
>  @@ -19,6 +19,7 @@
>   #include <asm/io.h>
>   #include <asm/paravirt.h>
>
>  +#ifdef CONFIG_PCI
>   /*
>   * Interrupt control on vSMPowered systems:
>   * ~AC is a shadow of IF.  If IF is 'on' AC should be 'off'
>  @@ -87,12 +88,18 @@ int is_vsmp_box(void)
>         if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
>              (PCI_VENDOR_ID_SCALEMP || (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
>                 vsmp = 1;
>  -
>         return vsmp;
>   }
>  +#else
>  +int is_vsmp_box(void)
>  +{
>  +       return 0;
>  +}
>  +#endif
>
>   void __init vsmp_init(void)
>   {
>  +#ifdef CONFIG_PCI
>         void *address;
>         unsigned int cap, ctl, cfg;
>
>  @@ -125,5 +132,6 @@ void __init vsmp_init(void)
>         }
>
>         early_iounmap(address, 8);
>  +#endif
>         return;
>   }
>  _

in arch/x86/Kconfig

config PCI
        bool "PCI support" if !X86_VISWS && !X86_VSMP
        depends on !X86_VOYAGER
        default y
        select ARCH_SUPPORTS_MSI if (X86_LOCAL_APIC && X86_IO_APIC)
        help
          Find out whether you have a PCI motherboard. PCI is the name of a
          bus system, i.e. the way the CPU talks to the other stuff inside
          your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or
          VESA. If you have PCI, say Y, otherwise N.

will make sure if X86_VSMP, the PCI=y

so NAK this patch.

YH

Re: + x86-vsmp-build-fixes.patch added to -mm tree

[Ravikiran Thirumalai]

On Mon, Mar 17, 2008 at 10:26:55PM -0700, Yinghai Lu wrote:
>On Mon, Mar 17, 2008 at 10:14 PM,  <akpm@linux-foundation.org> wrote:
>> ...
>>  Subject: x86: vsmp build fixes
>>  From: Glauber Costa <gcosta@redhat.com>
>>
>>  VSMP depends on PCI, but the file is now compiled conditionally on PARAVIRT,
>>  no VSMP, so enclose everything in an ifdef CONFIG_PCI.

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
>
>in arch/x86/Kconfig
>
>config PCI
>        bool "PCI support" if !X86_VISWS && !X86_VSMP
>        depends on !X86_VOYAGER
>        default y
>        select ARCH_SUPPORTS_MSI if (X86_LOCAL_APIC && X86_IO_APIC)
>        help
>          Find out whether you have a PCI motherboard. PCI is the name of a
>          bus system, i.e. the way the CPU talks to the other stuff inside
>          your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or
>          VESA. If you have PCI, say Y, otherwise N.
>
>will make sure if X86_VSMP, the PCI=y
>
>so NAK this patch.
>

Not really, as you can tell from the description Glauber provided above.

Btw, I had something similar to this cooking for the build breakage, along
with a few more updates (is_vsmp_box() is broken on vsmp).  Thanks Glauber
for this patch.

Thanks,
Kiran

Re: + x86-vsmp-build-fixes.patch added to -mm tree

[Yinghai Lu]

On Tue, Mar 18, 2008 at 12:28 AM, Ravikiran Thirumalai
<kiran@scalemp.com> wrote:
> On Mon, Mar 17, 2008 at 10:26:55PM -0700, Yinghai Lu wrote:
>  >On Mon, Mar 17, 2008 at 10:14 PM,  <akpm@linux-foundation.org> wrote:
>  >> ...
>
> >>  Subject: x86: vsmp build fixes
>  >>  From: Glauber Costa <gcosta@redhat.com>
>  >>
>  >>  VSMP depends on PCI, but the file is now compiled conditionally on PARAVIRT,
>  >>  no VSMP, so enclose everything in an ifdef CONFIG_PCI.
>
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  ...
>
> >
>  >in arch/x86/Kconfig
>  >
>  >config PCI
>  >        bool "PCI support" if !X86_VISWS && !X86_VSMP
>  >        depends on !X86_VOYAGER
>  >        default y
>  >        select ARCH_SUPPORTS_MSI if (X86_LOCAL_APIC && X86_IO_APIC)
>  >        help
>  >          Find out whether you have a PCI motherboard. PCI is the name of a
>  >          bus system, i.e. the way the CPU talks to the other stuff inside
>  >          your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or
>  >          VESA. If you have PCI, say Y, otherwise N.
>  >
>  >will make sure if X86_VSMP, the PCI=y
>  >
>  >so NAK this patch.
>  >
>
>  Not really, as you can tell from the description Glauber provided above.

oh, you mean PARAVIRT is defined, but VSMP is not used ?

then that patch is OK, but it is some ugly... or you can split that file?

>
>  Btw, I had something similar to this cooking for the build breakage, along
>  with a few more updates (is_vsmp_box() is broken on vsmp).  Thanks Glauber
>  for this patch.
>
that is funny, i just extracted is_vsmp_box from vsmp_init...

YH

Re: + x86-vsmp-build-fixes.patch added to -mm tree

[Yinghai Lu]

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

On Tue, Mar 18, 2008 at 12:28 AM, Ravikiran Thirumalai
<kiran@scalemp.com> wrote:
> On Mon, Mar 17, 2008 at 10:26:55PM -0700, Yinghai Lu wrote:
>  >On Mon, Mar 17, 2008 at 10:14 PM,  <akpm@linux-foundation.org> wrote:
>  >> ...
>
> >>  Subject: x86: vsmp build fixes
>  >>  From: Glauber Costa <gcosta@redhat.com>
>  >>
>  >>  VSMP depends on PCI, but the file is now compiled conditionally on PARAVIRT,
>  >>  no VSMP, so enclose everything in an ifdef CONFIG_PCI.
>
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  ...
>
> >
>  >in arch/x86/Kconfig
>  >
>  >config PCI
>  >        bool "PCI support" if !X86_VISWS && !X86_VSMP
>  >        depends on !X86_VOYAGER
>  >        default y
>  >        select ARCH_SUPPORTS_MSI if (X86_LOCAL_APIC && X86_IO_APIC)
>  >        help
>  >          Find out whether you have a PCI motherboard. PCI is the name of a
>  >          bus system, i.e. the way the CPU talks to the other stuff inside
>  >          your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or
>  >          VESA. If you have PCI, say Y, otherwise N.
>  >
>  >will make sure if X86_VSMP, the PCI=y
>  >
>  >so NAK this patch.
>  >
>
>  Not really, as you can tell from the description Glauber provided above.
>
suggest you change vsmp_64.c still depend to CONFIG_VSMP, and add some
inline function in some header file.
sth like attach patch

YH

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vsmp_init.patch --]
[-- Type: text/x-patch; name=vsmp_init.patch, Size: 1637 bytes --]

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 09b7b4e..00c0010 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -61,7 +61,7 @@ obj-$(CONFIG_KEXEC)		+= relocate_kernel_$(BITS).o crash.o
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump_$(BITS).o
 obj-$(CONFIG_X86_NUMAQ)		+= numaq_32.o
 obj-$(CONFIG_X86_SUMMIT_NUMA)	+= summit_32.o
-obj-$(CONFIG_PARAVIRT)		+= vsmp_64.o
+obj-$(CONFIG_X86_VSMP)		+= vsmp_64.o
 obj-$(CONFIG_KPROBES)		+= kprobes.o
 obj-$(CONFIG_MODULES)		+= module_$(BITS).o
 obj-$(CONFIG_ACPI_SRAT) 	+= srat_32.o
diff --git a/include/asm-x86/apic.h b/include/asm-x86/apic.h
index db5f750..6bcf559 100644
--- a/include/asm-x86/apic.h
+++ b/include/asm-x86/apic.h
@@ -51,13 +51,17 @@ extern unsigned boot_cpu_id;
  */
 #ifdef CONFIG_PARAVIRT
 #include <asm/paravirt.h>
-extern int is_vsmp_box(void);
 #else
 #define apic_write native_apic_write
 #define apic_write_atomic native_apic_write_atomic
 #define apic_read native_apic_read
 #define setup_boot_clock setup_boot_APIC_clock
 #define setup_secondary_clock setup_secondary_APIC_clock
+#endif
+
+#ifdef CONFIG_X86_VSMP
+extern int is_vsmp_box(void);
+#else
 static int inline is_vsmp_box(void)
 {
 	return 0;
diff --git a/include/asm-x86/setup.h b/include/asm-x86/setup.h
index f745de2..8516550 100644
--- a/include/asm-x86/setup.h
+++ b/include/asm-x86/setup.h
@@ -6,7 +6,13 @@
 #ifndef __ASSEMBLY__
 
 /* Interrupt control for vSMPowered x86_64 systems */
+#ifdef CONFIG_X86_VSMP
 void vsmp_init(void);
+#else
+static inline void vsmp_init(void)
+{
+}
+#endif
 
 char *machine_specific_memory_setup(void);
 #ifndef CONFIG_PARAVIRT

Fwd: + x86-vsmp-build-fixes.patch added to -mm tree

[Yinghai Lu]

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

add akpm


---------- Forwarded message ----------
From: Yinghai Lu <yhlu.kernel@gmail.com>
Date: Tue, Mar 18, 2008 at 1:25 AM
Subject: Re: + x86-vsmp-build-fixes.patch added to -mm tree
To: Ravikiran Thirumalai <kiran@scalemp.com>
Cc: linux-kernel@vger.kernel.org, mm-commits@vger.kernel.org,
gcosta@redhat.com, mingo@elte.hu, tglx@linutronix.de


On Tue, Mar 18, 2008 at 12:28 AM, Ravikiran Thirumalai
 <kiran@scalemp.com> wrote:
 > On Mon, Mar 17, 2008 at 10:26:55PM -0700, Yinghai Lu wrote:
 >  >On Mon, Mar 17, 2008 at 10:14 PM,  <akpm@linux-foundation.org> wrote:
 >  >> ...
 >
 > >>  Subject: x86: vsmp build fixes
 >  >>  From: Glauber Costa <gcosta@redhat.com>
 >  >>
 >  >>  VSMP depends on PCI, but the file is now compiled
conditionally on PARAVIRT,
 >  >>  no VSMP, so enclose everything in an ifdef CONFIG_PCI.
 >
 >  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 >  ...
 >
 > >
 >  >in arch/x86/Kconfig
 >  >
 >  >config PCI
 >  >        bool "PCI support" if !X86_VISWS && !X86_VSMP
 >  >        depends on !X86_VOYAGER
 >  >        default y
 >  >        select ARCH_SUPPORTS_MSI if (X86_LOCAL_APIC && X86_IO_APIC)
 >  >        help
 >  >          Find out whether you have a PCI motherboard. PCI is the name of a
 >  >          bus system, i.e. the way the CPU talks to the other stuff inside
 >  >          your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or
 >  >          VESA. If you have PCI, say Y, otherwise N.
 >  >
 >  >will make sure if X86_VSMP, the PCI=y
 >  >
 >  >so NAK this patch.
 >  >
 >
 >  Not really, as you can tell from the description Glauber provided above.
 >
 suggest you change vsmp_64.c still depend to CONFIG_VSMP, and add some
 inline function in some header file.
 sth like attach patch

 YH

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vsmp_init.patch --]
[-- Type: text/x-patch; name=vsmp_init.patch, Size: 1637 bytes --]

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 09b7b4e..00c0010 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -61,7 +61,7 @@ obj-$(CONFIG_KEXEC)		+= relocate_kernel_$(BITS).o crash.o
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump_$(BITS).o
 obj-$(CONFIG_X86_NUMAQ)		+= numaq_32.o
 obj-$(CONFIG_X86_SUMMIT_NUMA)	+= summit_32.o
-obj-$(CONFIG_PARAVIRT)		+= vsmp_64.o
+obj-$(CONFIG_X86_VSMP)		+= vsmp_64.o
 obj-$(CONFIG_KPROBES)		+= kprobes.o
 obj-$(CONFIG_MODULES)		+= module_$(BITS).o
 obj-$(CONFIG_ACPI_SRAT) 	+= srat_32.o
diff --git a/include/asm-x86/apic.h b/include/asm-x86/apic.h
index db5f750..6bcf559 100644
--- a/include/asm-x86/apic.h
+++ b/include/asm-x86/apic.h
@@ -51,13 +51,17 @@ extern unsigned boot_cpu_id;
  */
 #ifdef CONFIG_PARAVIRT
 #include <asm/paravirt.h>
-extern int is_vsmp_box(void);
 #else
 #define apic_write native_apic_write
 #define apic_write_atomic native_apic_write_atomic
 #define apic_read native_apic_read
 #define setup_boot_clock setup_boot_APIC_clock
 #define setup_secondary_clock setup_secondary_APIC_clock
+#endif
+
+#ifdef CONFIG_X86_VSMP
+extern int is_vsmp_box(void);
+#else
 static int inline is_vsmp_box(void)
 {
 	return 0;
diff --git a/include/asm-x86/setup.h b/include/asm-x86/setup.h
index f745de2..8516550 100644
--- a/include/asm-x86/setup.h
+++ b/include/asm-x86/setup.h
@@ -6,7 +6,13 @@
 #ifndef __ASSEMBLY__
 
 /* Interrupt control for vSMPowered x86_64 systems */
+#ifdef CONFIG_X86_VSMP
 void vsmp_init(void);
+#else
+static inline void vsmp_init(void)
+{
+}
+#endif
 
 char *machine_specific_memory_setup(void);
 #ifndef CONFIG_PARAVIRT

Re: + x86-vsmp-build-fixes.patch added to -mm tree

[Glauber Costa]

Yinghai Lu wrote:
> On Mon, Mar 17, 2008 at 10:14 PM,  <akpm@linux-foundation.org> wrote:
>>  The patch titled
>>      x86: vsmp build fixes
>>  has been added to the -mm tree.  Its filename is
>>      x86-vsmp-build-fixes.patch
>>
>>  Before you just go and hit "reply", please:
>>    a) Consider who else should be cc'ed
>>    b) Prefer to cc a suitable mailing list as well
>>    c) Ideally: find the original patch on the mailing list and do a
>>       reply-to-all to that, adding suitable additional cc's
>>
>>  *** Remember to use Documentation/SubmitChecklist when testing your code ***
>>
>>  See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find
>>  out what to do about this
>>
>>  The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>>
>>  ------------------------------------------------------
>>  Subject: x86: vsmp build fixes
>>  From: Glauber Costa <gcosta@redhat.com>
>>
>>  VSMP depends on PCI, but the file is now compiled conditionally on PARAVIRT,
>>  no VSMP, so enclose everything in an ifdef CONFIG_PCI.
>>
>>  We have to be careful enough to let is_vsmp_box and vsmp_init defined, since
>>  they are used outside this file
>>
>>  Signed-off-by: Glauber Costa <gcosta@redhat.com>
>>  Cc: Ravikiran Thirumalai <kiran@scalemp.com>
>>  Cc: Ingo Molnar <mingo@elte.hu>
>>  Cc: Thomas Gleixner <tglx@linutronix.de>
>>  Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>  ---
>>
>>   arch/x86/kernel/vsmp_64.c |   10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>>  diff -puN arch/x86/kernel/vsmp_64.c~x86-vsmp-build-fixes arch/x86/kernel/vsmp_64.c
>>  --- a/arch/x86/kernel/vsmp_64.c~x86-vsmp-build-fixes
>>  +++ a/arch/x86/kernel/vsmp_64.c
>>  @@ -19,6 +19,7 @@
>>   #include <asm/io.h>
>>   #include <asm/paravirt.h>
>>
>>  +#ifdef CONFIG_PCI
>>   /*
>>   * Interrupt control on vSMPowered systems:
>>   * ~AC is a shadow of IF.  If IF is 'on' AC should be 'off'
>>  @@ -87,12 +88,18 @@ int is_vsmp_box(void)
>>         if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
>>              (PCI_VENDOR_ID_SCALEMP || (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
>>                 vsmp = 1;
>>  -
>>         return vsmp;
>>   }
>>  +#else
>>  +int is_vsmp_box(void)
>>  +{
>>  +       return 0;
>>  +}
>>  +#endif
>>
>>   void __init vsmp_init(void)
>>   {
>>  +#ifdef CONFIG_PCI
>>         void *address;
>>         unsigned int cap, ctl, cfg;
>>
>>  @@ -125,5 +132,6 @@ void __init vsmp_init(void)
>>         }
>>
>>         early_iounmap(address, 8);
>>  +#endif
>>         return;
>>   }
>>  _
> 
> in arch/x86/Kconfig
> 
> config PCI
>         bool "PCI support" if !X86_VISWS && !X86_VSMP
>         depends on !X86_VOYAGER
>         default y
>         select ARCH_SUPPORTS_MSI if (X86_LOCAL_APIC && X86_IO_APIC)
>         help
>           Find out whether you have a PCI motherboard. PCI is the name of a
>           bus system, i.e. the way the CPU talks to the other stuff inside
>           your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or
>           VESA. If you have PCI, say Y, otherwise N.
> 
> will make sure if X86_VSMP, the PCI=y
Unfortunately, as said in the changelog, this file is compiled in 
conditionaly with PARAVIRT, not VSMP.

That's precisely the reason for the discovery code. Finding out wheter 
or not we're running under vsmp.