lkml.org 
[lkml]   [2008]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [patch 2/4] x86: Fix build breakage when PCI is define and PARAVIRT is not
    Ravikiran G Thirumalai wrote:
    > - Fix the the build breakage when PARAVIRT is defined
    > but PCI is not
    > This fixes problem reported at:
    > http://marc.info/?l=linux-kernel&m=120525966600698&w=2
    > - Make is_vsmp_box() available even when PARAVIRT is not defined.
    > This is needed to determine if tsc's are reliable as a time source
    > even when PARAVIRT is not defined.
    > - split vsmp_init to use is_vsmp_box() and set_vsmp_pv_ops()
    > set_vsmp_pv_ops will do nothing if PCI is not enabled in the config.
    >

    I'm a bit confused by all the config dependencies. I had assumed that
    VSMP depended on both PCI and PARAVIRT. Is this not actually true? You
    can have a VSMP system without either or both of PCI/PARAVIRT?

    The structure of the code suggests that at the very least VSMP depends
    on PCI (it never makes sense to enable VSMP on a non-PCI system), and
    therefore a number of your config decisions can just be predicated on VSMP.

    > Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
    >
    > Index: linux.git.trees/arch/x86/kernel/Makefile
    > ===================================================================
    > --- linux.git.trees.orig/arch/x86/kernel/Makefile 2008-03-19 13:39:29.000000000 -0700
    > +++ linux.git.trees/arch/x86/kernel/Makefile 2008-03-19 13:46:14.400935607 -0700
    > @@ -60,7 +60,7 @@ obj-$(CONFIG_KEXEC) += relocate_kernel_
    > 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-y += vsmp_64.o
    >

    Couldn't this be make obj-$(CONFIG_X86_VSMP)?

    > obj-$(CONFIG_KPROBES) += kprobes.o
    > obj-$(CONFIG_MODULES) += module_$(BITS).o
    > obj-$(CONFIG_ACPI_SRAT) += srat_32.o
    > Index: linux.git.trees/arch/x86/kernel/setup_64.c
    > ===================================================================
    > --- linux.git.trees.orig/arch/x86/kernel/setup_64.c 2008-03-19 13:39:29.000000000 -0700
    > +++ linux.git.trees/arch/x86/kernel/setup_64.c 2008-03-19 13:57:43.951337318 -0700
    > @@ -353,9 +353,7 @@ void __init setup_arch(char **cmdline_p)
    > if (efi_enabled)
    > efi_init();
    >
    > -#ifdef CONFIG_PARAVIRT
    > vsmp_init();
    > -#endif
    >
    > dmi_scan_machine();
    >
    > Index: linux.git.trees/arch/x86/kernel/vsmp_64.c
    > ===================================================================
    > --- linux.git.trees.orig/arch/x86/kernel/vsmp_64.c 2008-03-19 13:39:50.000000000 -0700
    > +++ linux.git.trees/arch/x86/kernel/vsmp_64.c 2008-03-19 20:53:34.267707163 -0700
    > @@ -19,6 +19,7 @@
    > #include <asm/io.h>
    > #include <asm/paravirt.h>
    >
    > +#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'
    > @@ -72,39 +73,11 @@ static unsigned __init vsmp_patch(u8 typ
    >
    > }
    >
    > -static int vsmp = -1;
    > -
    > -int is_vsmp_box(void)
    > -{
    > - if (vsmp != -1)
    > - return vsmp;
    > -
    > - vsmp = 0;
    > - if (!early_pci_allowed())
    > - return vsmp;
    > -
    > - /* Check if we are running on a ScaleMP vSMP box */
    > - if ((read_pci_config_16(0, 0x1f, 0, PCI_VENDOR_ID) ==
    > - PCI_VENDOR_ID_SCALEMP) &&
    > - (read_pci_config_16(0, 0x1f, 0, PCI_DEVICE_ID) ==
    > - PCI_DEVICE_ID_SCALEMP_VSMP_CTL))
    > - vsmp = 1;
    > -
    > - return vsmp;
    > -}
    > -
    > -void __init vsmp_init(void)
    > +static void __init set_vsmp_pv_ops(void)
    > {
    > void *address;
    > unsigned int cap, ctl, cfg;
    >
    > - if (!is_vsmp_box())
    > - return;
    > -
    > - if (!early_pci_allowed())
    > - return;
    > -
    > - /* If we are, use the distinguished irq functions */
    > pv_irq_ops.irq_disable = vsmp_irq_disable;
    > pv_irq_ops.irq_enable = vsmp_irq_enable;
    > pv_irq_ops.save_fl = vsmp_save_fl;
    > @@ -127,5 +100,46 @@ void __init vsmp_init(void)
    > }
    >
    > early_iounmap(address, 8);
    > +}
    > +#else
    > +static void __init set_vsmp_pv_ops(void)
    > +{
    > +}
    > +#endif
    > +
    > +#ifdef CONFIG_PCI
    > +static int vsmp = -1;
    > +
    > +int is_vsmp_box(void)
    > +{
    > + if (vsmp != -1)
    > + return vsmp;
    > +
    > + vsmp = 0;
    > + if (!early_pci_allowed())
    > + return vsmp;
    > +
    > + /* Check if we are running on a ScaleMP vSMP box */
    > + if ((read_pci_config_16(0, 0x1f, 0, PCI_VENDOR_ID) ==
    > + PCI_VENDOR_ID_SCALEMP) &&
    > + (read_pci_config_16(0, 0x1f, 0, PCI_DEVICE_ID) ==
    > + PCI_DEVICE_ID_SCALEMP_VSMP_CTL))
    > + vsmp = 1;
    > +
    > + return vsmp;
    > +}
    > +#else
    > +int is_vsmp_box(void)
    > +{
    > + return 0;
    > +}
    > +#endif
    >

    Rather than doing this, I'd propose putting

    #ifndef CONFIG_X86_VSMP
    static inline int is_vsmp_box(void)
    {
    return 0;
    }
    #endif

    in a header, and then making the definition in vsmp_64.c unconditional
    (or rather, the whole of vsmp_64.o conditional on CONFIG_X86_VSMP).

    > +
    > +void __init vsmp_init(void)
    > +{
    > + if (!is_vsmp_box())
    > + return;
    > +
    > + set_vsmp_pv_ops();
    > return;
    > }
    >

    Similarly with this.

    > Index: linux.git.trees/include/asm-x86/apic.h
    > ===================================================================
    > --- linux.git.trees.orig/include/asm-x86/apic.h 2008-03-19 13:39:29.000000000 -0700
    > +++ linux.git.trees/include/asm-x86/apic.h 2008-03-19 13:57:14.550893819 -0700
    > @@ -51,19 +51,16 @@ 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
    > -static int inline is_vsmp_box(void)
    > -{
    > - return 0;
    > -}
    > #endif
    >
    > +extern int is_vsmp_box(void);
    > +
    >

    This was almost right, except it should have been in a #ifdef
    CONFIG_X86_VSMP block.

    J


    \
     
     \ /
      Last update: 2008-03-21 05:37    [W:0.038 / U:0.532 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site