[Skiboot] [PATCH] Add nest_memory region to exports node

[Madhavan Srinivasan]

On Tuesday 12 September 2017 10:37 AM, Oliver wrote:
> On Tue, Sep 12, 2017 at 3:02 PM, Madhavan Srinivasan
> <maddy at linux.vnet.ibm.com> wrote:
>>
>> On Tuesday 12 September 2017 10:20 AM, Oliver wrote:
>>> On Fri, Sep 8, 2017 at 9:56 PM, Anju T Sudhakar <anju at linux.vnet.ibm.com>
>>> wrote:
>>>> From: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
>>>>
>>>> Adds nest_memory region to exports node, so that sysfs interface exposes
>>>> the nest-imc memory dump. This is really helpful incase of debugging
>>>> nest-imc
>>>> counters data and verify whether the counter values are updated in the
>>>> main
>>>> memory properly or not.
>>>> Also helps to analyse the values populated in the control block
>>>> structure.
>>>>
>>>> Example:
>>>>
>>>> /proc/device-tree/ibm,opal/firmware/exports# ls
>>>> hdat_map  imc_nest_chip_0  name  phandle  symbol_map
>>>>
>>>> /sys/firmware/opal/exports# ls
>>>> hdat_map  imc_nest_chip_0  symbol_map
>>>>
>>>> /sys/firmware/opal/exports# hexdump imc_nest_chip_0
>>>> 0000000 0000 0000 0000 47af 0000 0000 0000 b41e
>>>> 0000010 0000 0000 0400 7798 0000 0000 0000 0f6c
>>>> 0000020 0000 0000 0000 0000 0000 0000 0000 c206
>>>> 0000030 0000 0000 0000 f17b 0000 0000 0000 9100
>>>> 0000040 0000 0000 0000 0000 0000 0000 0000 b71e
>>>> 0000050 0000 0000 0400 6f98 0000 0000 0000 fc2b
>>>> ***
>>>>
>>>> Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
>>>> Tested-by : Anju T Sudhakar <anju at linux.vnet.ibm.com>
>>>> ---
>>>>    hw/imc.c | 30 ++++++++++++++++++++++++++++++
>>>>    1 file changed, 30 insertions(+)
>>>>
>>>> diff --git a/hw/imc.c b/hw/imc.c
>>>> index b29a2a1ba270..11df258601d1 100644
>>>> --- a/hw/imc.c
>>>> +++ b/hw/imc.c
>>>> @@ -113,6 +113,7 @@ static char *compress_buf;
>>>>    static size_t compress_buf_size;
>>>>    const char **prop_to_fix(struct dt_node *node);
>>>>    const char *props_to_fix[] = {"events", NULL};
>>>> +static uint32_t imc_nest_offset;
>>>>
>>>>    static bool is_nest_mem_initialized(struct imc_chip_cb *ptr)
>>>>    {
>>>> @@ -416,6 +417,33 @@ static void imc_dt_update_nest_node(struct dt_node
>>>> *dev)
>>>>           }
>>>>    }
>>>>
>>>> +static void add_nest_mem_exports_node(struct dt_node *root, struct
>>>> dt_node *dev)
>>>> +{
>>>> +       struct proc_chip *chip;
>>>> +       struct dt_node *node;
>>>> +       uint64_t baddr;
>>>> +       const struct dt_property *type;
>>>> +       char namebuf[32];
>>>> +       int i=0;
>>>> +
>>>> +       dt_for_each_compatible(dev, node, "ibm,imc-counters") {
>>>> +               type = dt_find_property(node, "type");
>>>> +               if (type && is_nest_node(node))
>>>> +                       imc_nest_offset = dt_prop_get_u32(node,
>>>> "offset");
>>>> +       }
>>>> +
>>>> +       node = dt_find_by_name(root, "exports");
>>> You could remove the param "root" from this function and rewrite it as:
>>>
>>>        dt_find_by_name(dt_opal, "exports");
>>
>> Yes, make sense. B/w you mean "dt_root" right?
> No, but dt_root would also work. dt_opal refers to the /ibm,opal/ node
> so it'll be slightly faster since there's a smaller search space. It's
Yep. Thats right.

> possible we might add another node caleld "exports" elsewhere in the
> tree some day so using dt_opal is probably safer too.

Ok thanks for details. Will make change.

Maddy

>
>>>> +       if (!node)
>>>> +               return;
>>>> +
>>>> +       for_each_chip(chip) {
>>>> +               snprintf(namebuf, sizeof(namebuf), "imc_nest_chip_%d",
>>>> i++);
>>> Can you use the chip_id (in hex) here rather than a counter?
>>
>> sure.
>>
>>>> +               baddr = chip->homer_base;
>>>> +               baddr += imc_nest_offset;
>>>> +               dt_add_property_u64s(node, namebuf, baddr, 0x40000);
>>> Why is the size hard coded rather than using the imc-nest-size property?
>>
>> Yep fixed it in v2 of the patch
>>
>> Thanks for the review
> no worries.
>
>> Maddy
>>
>>
>>>> +       }
>>>> +}
>>>> +
>>>>    /*
>>>>     * Load the IMC pnor partition and find the appropriate sub-partition
>>>>     * based on the platform's PVR.
>>>> @@ -522,6 +550,8 @@ imc_mambo:
>>>>                   goto err;
>>>>           }
>>>>
>>>> +       add_nest_mem_exports_node(dt_root, dev);
>>>> +
>>>>           free(compress_buf);
>>>>           return;
>>>>    err:
>>>> --
>>>> 2.7.4
>>>>
>>>> _______________________________________________
>>>> Skiboot mailing list
>>>> Skiboot at lists.ozlabs.org
>>>> https://lists.ozlabs.org/listinfo/skiboot
>>