[2/2] Update ibm,client-architecture call field based on device tree

In the previous patch the client-architecture field for the number of
cores supported is set statically as high as is possible.  However, that
static setting could be too high if the system supports smt, resulting
in cpus assigned to Linux that are not booted.  This patch reads the
device tree (before it is unflattened) to determine the amount of smt.
It then dynamically updates the entires in the array with the proper
number of cores supported.  Tests show this correctly detecting SMT4 on
a Power7 and still booting all the supported cores on a large machine.


Signed-off-by:Joel Schopp<jschopp@austin.ibm.com>

On Thu, 2010-01-14 at 14:41 -0600, Joel Schopp wrote:
> In the previous patch the client-architecture field for the number of
> cores supported is set statically as high as is possible.  However, that
> static setting could be too high if the system supports smt, resulting
> in cpus assigned to Linux that are not booted.  This patch reads the
> device tree (before it is unflattened) to determine the amount of smt.
> It then dynamically updates the entires in the array with the proper
> number of cores supported.  Tests show this correctly detecting SMT4 on
> a Power7 and still booting all the supported cores on a large machine.

Same comments about submission format as the previous patch...

> Signed-off-by:Joel Schopp<jschopp@austin.ibm.com> 
> Index: linux-2.6.git/arch/powerpc/kernel/prom_init.c
> ===================================================================
> --- linux-2.6.git.orig/arch/powerpc/kernel/prom_init.c
> +++ linux-2.6.git/arch/powerpc/kernel/prom_init.c
> @@ -141,6 +141,8 @@ typedef u32 cell_t;
>  
>  extern void __start(unsigned long r3, unsigned long r4, unsigned long r5);
>  
> +static int __init prom_smt_way(void);

Do you really need a forward declaration ? We generally avoid those and
prefer instead having the functions in the right order to make it
unnecessary.

>  #ifdef CONFIG_PPC64
>  extern int enter_prom(struct prom_args *args, unsigned long entry);
>  #else
> @@ -811,9 +813,17 @@ static void __init prom_send_capabilitie
>  {
>  	ihandle elfloader, root;
>  	prom_arg_t ret;
> +	u32 *cores;
>  
>  	root = call_prom("open", 1, 1, ADDR("/"));
>  	if (root != 0) {
> +		/*
> +		 * If you add to the struct, please be sure the 100 index
> +		 * didn't change.  The BUILD_BUG_ON is a reminder.
> +		 */
> +		BUILD_BUG_ON(sizeof(ibm_architecture_vec) != 108);

This is indeed a bit fishy... a nicer way may have been to have the
vector in an asm file with labels but that's probably overkill. Just
maybe add a runtime test that checks the value read initially is
NR_CORES and if not, print a big fat warning ?

> +		cores = (u32 *) &ibm_architecture_vec[100];
> +		*cores = (u32) (NR_CPUS/prom_smt_way());

The style is a bit gross though I suppose it will do. Drop the cast on
the second line, it's not useful, and stick some spaces around that
division. Also maybe print out a message saying to what value you
adjusted the max number of supported cores, might be useful on the field
to diagnose issues.

>  		/* try calling the ibm,client-architecture-support method */
>  		prom_printf("Calling ibm,client-architecture-support...");
>  		if (call_prom_ret("call-method", 3, 2, &ret,
> @@ -1031,6 +1041,45 @@ static void __init reserve_mem(u64 base,
>  	RELOC(mem_reserve_cnt) = cnt + 1;
>  }
>  
> +
> +static int __init prom_smt_way(void)
> +{
> +	phandle node;
> +	char type[64];
> +	unsigned int plen;
> +
> +	for (node = 0; prom_next_node(&node); ) {
> +		type[0] = 0;
> +		prom_getprop(node, "device_type", type, sizeof(type));
> +
> +		if (type[0] == 0) {
> +			/*
> +			 * CHRP Longtrail machines have no device_type
> +			 * on the memory node, so check the name instead...
> +			 */
> +			prom_getprop(node, "name", type, sizeof(type));

They also have no SMT :-) Just continue instead. You could also have
instead gone for /cpus and peeked at the first child. Might have lead to
a simpler construct (and faster runtime)

> +		}
> +		if (strcmp(type, RELOC("cpu")))
> +			continue;
> +
> +		/*
> +		 * There is an entry for each smt thread, each entry being
> +		 * 4 bytes long.  All cpus should have the same number of
> +		 * smt threads, so return after finding the first.
> +		 */
> +		plen = prom_getproplen(node, "ibm,ppc-interrupt-server#s");
> +		prom_debug("smt %x\n", (unsigned long) plen);

Might only be a debug message but it should be a tad more verbose about
what it's actually printing.

> +		if (plen >= 4)
> +			return plen / 4;
> +	}
> +	/*
> +	 * If things go wrong and we get here fallback to SMT1
> +	 */
> +	prom_debug("unable to determine smt from device tree, guessing smt1\n");
> +	return 1;
> +
> +}
> +

Cheers,
Ben.

>  /*
>   * Initialize memory allocation mechanism, parse "memory" nodes and
>   * obtain that way the top of memory and RMO to setup out local allocator
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev