RE: [PATCH v4] vsmp: Fix number of CPUs when vsmp is disabled

[Shai Fultheim (Shai@xxxxxxxxxxx)]

Looks good.


--Shai 


> -----Original Message-----
> From: Ido Yariv [mailto:ido@xxxxxxxxxx]
> Sent: Friday, May 11, 2012 12:27
> To: H. Peter Anvin
> Cc: Ingo Molnar; linux-kernel@xxxxxxxxxxxxxxx; Ingo Molnar; Thomas
> Gleixner; Shai Fultheim (Shai@xxxxxxxxxxx)
> Subject: Re: [PATCH v4] vsmp: Fix number of CPUs when vsmp is disabled
> 
> Hi,
> 
> On Wed, May 09, 2012 at 08:55:54AM -0700, H. Peter Anvin wrote:
> > On 05/09/2012 08:44 AM, Ingo Molnar wrote:
> > >
> > > * Ido Yariv <ido@xxxxxxxxxx> wrote:
> > >
> > >> +static void __init vsmp_cap_cpus(void)
> > >> +{
> > >> +	void __iomem *address;
> > >> +	unsigned int cfg, topology, node_shift, maxcpus;
> > >> +
> > >> +#ifdef CONFIG_X86_VSMP
> > >> +	/* VSMP is enabled, no need to cap cpus */
> > >> +	return;
> > >> +#elif defined(CONFIG_SMP)
> > >> +	/*
> > >> +	 * CONFIG_X86_VSMP is not configured, so limit the number CPUs to
> the
> > >
> > > I suspect this will throw compiler warnings in the
> > > CONFIG_X86_VSMP && !CONFIG_SMP case.
> > >
> >
> > What on Earth is the point of allowing that combination?  Why not make
> > X86_VSMP depend on SMP and reduce the testing matrix?
> 
> CONFIG_X86_VSMP shouldn't be used without CONFIG_SMP, so we can
> safely
> add this dependency.
> Ingo's suggestion will still fix unreferenced variables warnings, so how
> about the following patch?
> 
> Thanks,
> Ido.
> 
> From 6fa023be44749092088618379f13d7a3e086d692 Mon Sep 17 00:00:00
> 2001
> From: Shai Fultheim <shai@xxxxxxxxxxx>
> Date: Mon, 16 Apr 2012 10:39:35 +0300
> Subject: [PATCH v5] vsmp: Fix number of CPUs when vsmp is disabled
> 
> In case CONFIG_X86_VSMP is not set, limit the number of CPUs to the
> number of CPUs of the first board.
> 
> Also make CONFIG_X86_VSMP depend on CONFIG_SMP, as there's little
> point in having a vsmp machine with a single CPU.
> 
> Signed-off-by: Shai Fultheim <shai@xxxxxxxxxxx>
> [ido@xxxxxxxxxx: rebased, fixed minor coding-style issues]
> Signed-off-by: Ido Yariv <ido@xxxxxxxxxx>
> ---
>  arch/x86/Kconfig          |    1 +
>  arch/x86/kernel/vsmp_64.c |   40
> ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c9866b0..b1e98c9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -374,6 +374,7 @@ config X86_VSMP
>  	select PARAVIRT
>  	depends on X86_64 && PCI
>  	depends on X86_EXTENDED_PLATFORM
> +	depends on SMP
>  	---help---
>  	  Support for ScaleMP vSMP systems.  Say 'Y' here if this kernel is
>  	  supposed to run on these EM64T-based machines.  Only choose this
> option
> diff --git a/arch/x86/kernel/vsmp_64.c b/arch/x86/kernel/vsmp_64.c
> index a1d804b..8eeb55a 100644
> --- a/arch/x86/kernel/vsmp_64.c
> +++ b/arch/x86/kernel/vsmp_64.c
> @@ -15,6 +15,7 @@
>  #include <linux/init.h>
>  #include <linux/pci_ids.h>
>  #include <linux/pci_regs.h>
> +#include <linux/smp.h>
> 
>  #include <asm/apic.h>
>  #include <asm/pci-direct.h>
> @@ -22,6 +23,8 @@
>  #include <asm/paravirt.h>
>  #include <asm/setup.h>
> 
> +#define TOPOLOGY_REGISTER_OFFSET 0x10
> +
>  #if defined CONFIG_PCI && defined CONFIG_PARAVIRT
>  /*
>   * Interrupt control on vSMPowered systems:
> @@ -149,12 +152,49 @@ int is_vsmp_box(void)
>  	return 0;
>  }
>  #endif
> +
> +static void __init vsmp_cap_cpus(void)
> +{
> +#if !defined(CONFIG_X86_VSMP) && defined(CONFIG_SMP)
> +	void __iomem *address;
> +	unsigned int cfg, topology, node_shift, maxcpus;
> +
> +	/*
> +	 * CONFIG_X86_VSMP is not configured, so limit the number CPUs to
> the
> +	 * ones present in the first board, unless explicitly overridden by
> +	 * setup_max_cpus
> +	 */
> +	if (setup_max_cpus != NR_CPUS)
> +		return;
> +
> +	/* Read the vSMP Foundation topology register */
> +	cfg = read_pci_config(0, 0x1f, 0, PCI_BASE_ADDRESS_0);
> +	address = early_ioremap(cfg + TOPOLOGY_REGISTER_OFFSET, 4);
> +	if (WARN_ON(!address))
> +		return;
> +
> +	topology = readl(address);
> +	node_shift = (topology >> 16) & 0x7;
> +	if (!node_shift)
> +		/* The value 0 should be decoded as 8 */
> +		node_shift = 8;
> +	maxcpus = (topology & ((1 << node_shift) - 1)) + 1;
> +
> +	pr_info("vSMP CTL: Capping CPUs to %d (CONFIG_X86_VSMP is
> unset)\n",
> +		maxcpus);
> +	setup_max_cpus = maxcpus;
> +	early_iounmap(address, 4);
> +#endif
> +}
> +
>  void __init vsmp_init(void)
>  {
>  	detect_vsmp_box();
>  	if (!is_vsmp_box())
>  		return;
> 
> +	vsmp_cap_cpus();
> +
>  	set_vsmp_pv_ops();
>  	return;
>  }
> --
> 1.7.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/