Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ea3e6838-a656-9a20-8240-b312dd54f285@quicinc.com>
Date:   Thu, 19 Jan 2023 22:21:27 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.0
Subject: Re: [PATCH V0 1/1] bootconfig: Increase max size of bootconfig from
 32 KB to 256 KB for DCC support
Content-Language: en-US
To:     "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
CC:     <linux-kernel@vger.kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-arm-msm@vger.kernel.org>,
        "Sai Prakash Ranjan" <quic_saipraka@quicinc.com>,
        Sibi Sankar <quic_sibis@quicinc.com>,
        Rajendra Nayak <quic_rjendra@quicinc.com>
References: <cover.1673261071.git.quic_schowdhu@quicinc.com>
 <654357bcbfd3974072a558c494a51edafaa73e1a.1673261071.git.quic_schowdhu@quicinc.com>
 <20230110001820.5ca81344286f614ed4ccec77@kernel.org>
 <e2ac0fa4-28f0-f4d8-e02a-b2a5d6131a48@quicinc.com>
 <20230110234643.7bbd340ece99c28f25fe7ad7@kernel.org>
 <9545ca51-ccda-64f0-bdd4-3b53e06785ad@quicinc.com>
 <20230112160128.600f9e7257d67aa63a5fbcb9@kernel.org>
From:   Souradeep Chowdhury <quic_schowdhu@quicinc.com>
In-Reply-To: <20230112160128.600f9e7257d67aa63a5fbcb9@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 1/12/2023 12:31 PM, Masami Hiramatsu (Google) wrote:
> On Tue, 10 Jan 2023 20:38:54 +0530
> Souradeep Chowdhury <quic_schowdhu@quicinc.com> wrote:
> 
>>
>>
>> On 1/10/2023 8:16 PM, Masami Hiramatsu (Google) wrote:
>>> On Tue, 10 Jan 2023 17:26:07 +0530
>>> Souradeep Chowdhury <quic_schowdhu@quicinc.com> wrote:
>>>
>>>>
>>>>
>>>> On 1/9/2023 8:48 PM, Masami Hiramatsu (Google) wrote:
>>>>> On Mon, 9 Jan 2023 20:01:05 +0530
>>>>> Souradeep Chowdhury <quic_schowdhu@quicinc.com> wrote:
>>>>>
>>>>>> Increasing the memory size of bootconfig to be able to handle a max number of
>>>>>> 8192 nodes to be fitted in memory size of 256KB.
>>>>>
>>>>> Sorry, but you missed the 'xbc_node::data' stores the index of the data and
>>>>> that is uint16_t. So the XBC_DATA_MAX is fixed limitation.
>>>>>
>>>>> The number of nodes (XBC_NODE_MAX) can be expanded because I just decided it
>>>>> to keep the pre-compiled array size ~8KB. Maybe expanding it to 64KB just
>>>>> increase the size of kernel on init memory (and freed after boot).
>>>>>
>>>>> Could you tell me why you need such a big data for your DCC?
>>>>>
>>>>> Thank you,
>>>>
>>>> DCC is a debugging tool used in qcom which is needed to debug crashes
>>>> that can happen at boot-time. For debugging purposes a large number of
>>>> registers need to be configured in DCC driver which is to be fed via the
>>>> bootconfig file. For that we need to expand the nodes as well as memory
>>>> for using bootconfig.
>>>
>>> Hmm, how many registers does DCC usually use? And how big the bootconfig
>>> file is usually? I have no idea about that.
>>
>> So a typical bootconfig file for consumption of DCC looks like as follows
>>
>> dcc_config {
>>           link_list_0 {
>>                   qcom-curr-link-list = 6
>>                   qcom-link-list = R_0x1781005c_1_apb,
>>                                    R_0x1782005c_1_apb
>>           }
>>           link_list_1 {
>>                   qcom-curr-link-list = 5
>>                   qcom-link-list = R_0x1784005c_1_apb
>>           }
>> }
>>
>> The "qcom-link-list" field can have 1000s of register , based on that
>> max nodes is increased to 8192.
> 
> OK, then the number of fields can be larger than 1000. I got it.
> 
>>
>>>
>>>> Can you let us know the changes that you suggest for doing the same? Is
>>>> it fine to just increase the XBC_NODE_MAX, do we also need to
>>>> change the uint16_t to u32 for proper storing of index values?
>>>
>>> Expanding the number of max nodes is easy, just increase the XBC_NODE_MAX
>>> (must be less than 64k). That will also increase the memory consumption
>>> during the boot time even if the bootconfig is small. Anyway, it will be
>>> freed after boot, so it maybe OK.
>>
>> So since the limit is 64K, 8192 is a valid value for max nodes.
> 
> Yes. Expanding the number of node is OK to me.
> 
>>
>>>
>>> But expanding the size of max bootconfig needs to change the type of
>>> the 'data' field to uint32_t (since that will be used for building
>>> bootconfig tool) and you also must confirm that `tools/bootconfig/bootconfig`
>>> can be built and pass the test-bootconfig.sh.
>>> Hmm, comparing with expanding the max number of XBC node, changing the
>>> 'data' type to uint32_t may not have much impact on memory consumption point
>>> of view, because it may increase only 20% of memory, but expanding the
>>> MAX_XBC_NODE always increases more than double.
>>>
>>> Thus, if we can accept increasing the number of node, it should be OK to
>>> change the 'data' type.
>>
>> That means from DCC point of view only increasing the max nodes is
>> enough as increasing the data size is unrelated to increasing the max nodes?
> 
> Yes, if it is less than 32KB, you just need to increase the XBC_NODE_MAX.
> But if you think the size of bootconfig, we have to change the type of
> xbc_node::data field.
> 
> Can you check the DCC also need to expand the size of bootconfig limitation?
> 
> Thank you!

Yes, I don't think the index needs to be increased from u16 to u32 for 
dcc. Will be sending out the next version accordingly.

Thanks
> 
>>
>>>
>>> BTW, I think now we don't need the ' __attribute__ ((__packed__))' for
>>> struct xbc_node. It was packed for reducing the size of array and able to
>>> pass 'compiled' bootconfig, but now it is just passed as a text data for
>>> safety.
>>
>>>
>>> Thank you,
>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>>>>>> ---
>>>>>>     include/linux/bootconfig.h | 6 +++---
>>>>>>     1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
>>>>>> index 1611f9d..64d233b 100644
>>>>>> --- a/include/linux/bootconfig.h
>>>>>> +++ b/include/linux/bootconfig.h
>>>>>> @@ -55,11 +55,11 @@ struct xbc_node {
>>>>>>     } __attribute__ ((__packed__));
>>>>>>     
>>>>>>     #define XBC_KEY		0
>>>>>> -#define XBC_VALUE	(1 << 15)
>>>>>> -/* Maximum size of boot config is 32KB - 1 */
>>>>>> +#define XBC_VALUE	(1 << 18)
>>>>>> +/* Maximum size of boot config is 256KB - 1 */
>>>>>>     #define XBC_DATA_MAX	(XBC_VALUE - 1)
>>>>>>     
>>>>>> -#define XBC_NODE_MAX	1024
>>>>>> +#define XBC_NODE_MAX	8192
>>>>>>     #define XBC_KEYLEN_MAX	256
>>>>>>     #define XBC_DEPTH_MAX	16
>>>>>>     
>>>>>> -- 
>>>>>> 2.7.4
>>>>>>
>>>>>
>>>>>
>>>
>>>
> 
>