On Tue, 10 Jan 2023 20:38:54 +0530
Souradeep Chowdhury <quic_schowdhu@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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!
>
> >
> > 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@xxxxxxxxxxx>
> >>>> ---
> >>>> 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@xxxxxxxxxx>
