Re: [PATCH V0 1/1] bootconfig: Increase max size of bootconfig from 32 KB to 256 KB for DCC support
From: Souradeep Chowdhury
Date: Thu Jan 19 2023 - 11:52:49 EST
On 1/12/2023 12:31 PM, Masami Hiramatsu (Google) wrote:
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!
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@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