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;
Date:   Tue, 10 Jan 2023 23:46:43 +0900
From:   Masami Hiramatsu (Google) <mhiramat@kernel.org>
To:     Souradeep Chowdhury <quic_schowdhu@quicinc.com>
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>
Subject: Re: [PATCH V0 1/1] bootconfig: Increase max size of bootconfig from
 32 KB to 256 KB for DCC support
Message-Id: <20230110234643.7bbd340ece99c28f25fe7ad7@kernel.org>
In-Reply-To: <e2ac0fa4-28f0-f4d8-e02a-b2a5d6131a48@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>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.

> 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.

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.

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
> >>
> > 
> > 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>