Re: [PATCH] powerpc: update ibm,client-architecture

[Joel Schopp <jschopp@austin.ibm.com>]

Tony Breeds wrote:
> On Fri, Dec 18, 2009 at 03:07:32PM -0600, Joel Schopp wrote:
>   
>> In order to boot with more than 64 cores on machines that support the
>> ibm,client-architecture RTAS call a new field has been added to the
>> structure.  This patch updates that field and adds a few others in the
>> process.  It would be good if this could go in as a bugfix.  
>>     
>
> But it's not really is it? What does it fix?
>   
It fixes kernels compiled with NR_CPUS where NR_CPUS > 256. 
>
>>  	/* option vector 5: PAPR/OF options */
>> -	5 - 2,				/* length */
>> +	13 - 2,				/* length */
>>  	0,				/* don't ignore, don't halt */
>>  	OV5_LPAR | OV5_SPLPAR | OV5_LARGE_PAGES | OV5_DRCONF_MEMORY |
>>  	OV5_DONATE_DEDICATE_CPU | OV5_MSI,
>>  	0,
>>  	OV5_CMO,
>> +	0,				/* reserved */
>> +	0,				/* reserved */
>> +	0,				/* reserved */
>> +	0,				/* reserved */
>>     
>
> Not so much reserved, as unused by us.
>   
They were all reserved until recently, and the last 2 of the 4 are still 
reserved, while we just don't use the first two yet.  I'll remove the 
comments from the first 2.

>   
>> +	W(NR_CPUS/4),			/* max cores supported */
>>     
>
> 4?  I can see a case for 2 or just plain NR_CPUS, but 4 is wrong.
>   
4 is the new 2.  Since the actual threads per core is unknown at this 
point in boot you have to be conservative and go with the maximum number 
of any processor.  See page 4 of these charts:
http://www.power.org/events/powercon09/taiwan09/IBM_Overview_POWER7.pdf
>   
>> +	/* option vector 6: IBM PAPR hints */
>> +	4 - 2,
>>     
>
> We all know this is a length, but please follow the style in this structure.
>   
OK.
>   
>> +	0,				/* reserved */
>> +	0,				/* 1 == no secondary pteg */
>>     
>
> I think the "1 == " confuses things.
>   
I'll remove it then.
>   
>> +	OV6_LINUX,
>>     
>
> Does filling in the vector actually let us boot on systems where we do not already?
>   
Will add description to the email body on this one.
>   
>> +
>> +	/* option vector 7: IBM PAPR OS identification */
>> +	/* a human readable ascii string will go here */
>>     
>
> Either fill it in or leave the comments out.  
>   
I'll leave the comment out.