2008-03-18 05:35:56

by Yinghai Lu

[permalink] [raw]
Subject: Re: + x86-vsmp-build-fixes.patch added to -mm tree

On Mon, Mar 17, 2008 at 10:14 PM, <[email protected]> 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 <[email protected]>
>
> 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 <[email protected]>
> Cc: Ravikiran Thirumalai <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> 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


2008-03-18 07:28:19

by Ravikiran Thirumalai

[permalink] [raw]
Subject: Re: + x86-vsmp-build-fixes.patch added to -mm tree

On Mon, Mar 17, 2008 at 10:26:55PM -0700, Yinghai Lu wrote:
>On Mon, Mar 17, 2008 at 10:14 PM, <[email protected]> wrote:
>> ...
>> Subject: x86: vsmp build fixes
>> From: Glauber Costa <[email protected]>
>>
>> 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

2008-03-18 07:57:16

by Yinghai Lu

[permalink] [raw]
Subject: Re: + x86-vsmp-build-fixes.patch added to -mm tree

On Tue, Mar 18, 2008 at 12:28 AM, Ravikiran Thirumalai
<[email protected]> wrote:
> On Mon, Mar 17, 2008 at 10:26:55PM -0700, Yinghai Lu wrote:
> >On Mon, Mar 17, 2008 at 10:14 PM, <[email protected]> wrote:
> >> ...
>
> >> Subject: x86: vsmp build fixes
> >> From: Glauber Costa <[email protected]>
> >>
> >> 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

2008-03-18 08:25:26

by Yinghai Lu

[permalink] [raw]
Subject: Re: + x86-vsmp-build-fixes.patch added to -mm tree

On Tue, Mar 18, 2008 at 12:28 AM, Ravikiran Thirumalai
<[email protected]> wrote:
> On Mon, Mar 17, 2008 at 10:26:55PM -0700, Yinghai Lu wrote:
> >On Mon, Mar 17, 2008 at 10:14 PM, <[email protected]> wrote:
> >> ...
>
> >> Subject: x86: vsmp build fixes
> >> From: Glauber Costa <[email protected]>
> >>
> >> 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


Attachments:
(No filename) (1.35 kB)
vsmp_init.patch (1.60 kB)
Download all attachments

2008-03-18 08:37:57

by Yinghai Lu

[permalink] [raw]
Subject: Fwd: + x86-vsmp-build-fixes.patch added to -mm tree

add akpm


---------- Forwarded message ----------
From: Yinghai Lu <[email protected]>
Date: Tue, Mar 18, 2008 at 1:25 AM
Subject: Re: + x86-vsmp-build-fixes.patch added to -mm tree
To: Ravikiran Thirumalai <[email protected]>
Cc: [email protected], [email protected],
[email protected], [email protected], [email protected]


On Tue, Mar 18, 2008 at 12:28 AM, Ravikiran Thirumalai
<[email protected]> wrote:
> On Mon, Mar 17, 2008 at 10:26:55PM -0700, Yinghai Lu wrote:
> >On Mon, Mar 17, 2008 at 10:14 PM, <[email protected]> wrote:
> >> ...
>
> >> Subject: x86: vsmp build fixes
> >> From: Glauber Costa <[email protected]>
> >>
> >> 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


Attachments:
(No filename) (1.73 kB)
vsmp_init.patch (1.60 kB)
Download all attachments

2008-03-18 09:44:22

by Glauber Costa

[permalink] [raw]
Subject: Re: + x86-vsmp-build-fixes.patch added to -mm tree

Yinghai Lu wrote:
> On Mon, Mar 17, 2008 at 10:14 PM, <[email protected]> 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 <[email protected]>
>>
>> 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 <[email protected]>
>> Cc: Ravikiran Thirumalai <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Signed-off-by: Andrew Morton <[email protected]>
>> ---
>>
>> 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.