[Skiboot] [PATCH v11 03/11] skiboot: Add chip id to HOMER reserved region

[Vasant Hegde]

On 05/12/2017 06:50 AM, Stewart Smith wrote:
> Madhavan Srinivasan <maddy at linux.vnet.ibm.com> writes:
>> From: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>>
>> This patch adds a "ibm,chip-id" property to HOMER reserved region. That
>> helps us in relating the HOMER reserved region to the chip that its
>> associated with.
>>
>> This patch is required for nest IMC counters since, the counter values
>> are stored in an offset inside the HOMER region. To read those values
>> from the kernel for a chip, we need to associate the region to its
>> associated chip.
>>
>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
>> Signed-off-by: Hemant Kumar <hemant at linux.vnet.ibm.com>
>> Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
>> ---
>>  core/init.c       |  3 +++
>>  hw/homer.c        | 30 ++++++++++++++++++++++++++++++
>>  include/skiboot.h |  1 +
>>  3 files changed, 34 insertions(+)
>
> As mentioned in review of the kernel patch (see
> https://patchwork.ozlabs.org/patch/758583/ ) I think it woul be better
> that if instead of keying everything to /reserved-memory/ibm,homer-image
> we just provided a base address either directly in the IMC DT part or
> have a phandle reference to the reserved-memory region that's
> appropriate.

Stewart,
   My original idea was to just add property here (as we already have rest of 
the information under
/reserved-memory) and get done that. Also I don't foresee adding too many 
properties here.

   Adding another DT property is nice. All  my concern is , soon we will have 
similar requirement for
other nodes under /reserved.

ex: for SBE FFDC (where we need to capture SBE FFDC and put it on host), I need 
to know the chip-id
for each FFDC region.  Also RHB section of HDAT contains other information like 
type of reserved
memory (like TPM log). Others may be interested to get those information as 
well. How do we represent those information?

By continuing approach used in this patch, we will be done by adding few more 
property to reserved
memory node  and done with that. Since all these properties are related to 
reserved memory, IMO
its fine to add them under /reserved-memory. Let me know your thoughts on this 
approach.

-Vasant