| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 20 Mar 2008 21:30:09 -0700
From: Jeremy Fitzhardinge <jeremy@...p.org>
To: Ravikiran G Thirumalai <kiran@...lex86.org>
CC: Andrew Morton <akpm@...ux-foundation.org>,
Ingo Molnar <mingo@...e.hu>, linux-kernel@...r.kernel.org,
Glauber de Oliveira Costa <gcosta@...hat.com>,
Andi Kleen <ak@...e.de>, shai@...lex86.org
Subject: Re: [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@...lex86.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists