Message-Id: <20200716102703.33435594dbf192157cf9655f@kernel.org>
Date:   Thu, 16 Jul 2020 10:27:03 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Alistair Delva <adelva@...gle.com>, linux-doc@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] bootconfig: Add value override operator

Hi Steve,

On Wed, 15 Jul 2020 20:02:12 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> On Thu, 16 Jul 2020 07:38:43 +0900
> Masami Hiramatsu <mhiramat@...nel.org> wrote:
> 
> 
> > > So the end of the initrd would have:
> > > 
> > >  [data][size/checksum/magic][more-data][size/checksum/magic]
> > > 
> > > 
> > > And the kernel could do the following:
> > > 
> > >  1. read the end of the initrd for bootconfig
> > >  2. If found parse the bootconfig data.
> > >  3. look at the content before the bootconfig
> > >  4. if another bootconfig exists, goto 2.
> > >   
> > 
> > Yeah, that is possible. But since the total size of the bootconfig
> > is limited to 32KB (this means data + 1st footer + more-data),
> > I would like to give a chance of sanity check to the bootloader.
> 
> 
> That's a limit of the size field, right?

If you mean the size field in the footer, no, it is u32.

To be honest, the size limitation came from the xbc_node data
structure. To minimize the memory footprint, I decided to
pack the data structure into 64bits with 4 fields.
Each field has 16bits, and the data field needs 1 bit flag
to distinguish the value and the key.
Thus the maximum number of nodes can be expanded to 64K
(but it is not feasible, maybe less than 8K will be a real
size), but the data field (the offset from the bootconfig
start address) is 15bits = 32KB long.
Of course we can use the bitfield to tune it, but maybe current
balance ( 512 node / 32KB ) is enough.

Note that if we expand the number of nodes, we need to pre-allocate
the node data structure as a static data (in .bss) because parsing
will be done before initializing memory management. 512 nodes means
4096B is already allocated. 8K node needs 64KB, and that will be
not filled in most cases.

> The bootloader (and all tools including the kernel) could check for
> multiple instances, and that would even increase the size of what can
> be added. As each section would be 32KB max size, but there's no limit
> to how many you have. All tools, bootconfig, the bootloader, and the
> kernel can perform the checksum.

In fact, I previously considered the multi-section, but came to the
conclusion that it wasn't much benefit for both Linux and the
bootloaders.

If we support multi-section, we have to mix the section nodes on
a single tree for overriding values, this means the data field must
have a section number (and per-section starting address pointers),
or an offset from the 1st section address.

And I think it is easy for the bootloader to just concatenate the
data as below, because the data is already on memory.

[data][more-data][size/checksum/magic]

To support multiple-section, the bootloader will do

0. load the bootconfig with the initrd from media
1. prepare the data
2. calculate the size and checksum of the data
3. append the data and footer right after the last footer

and to support single section,

0. load the bootconfig with the initrd from media
1. prepare the data
2. calculate the size and checksum of the data
3. increment the size and the checksum 
   (note that the size and checksum is already on memory)
4. append the data and footer right after the last data

Thus, I think there is no much benefit to support multiple sections.

Thank you,

-- 
Masami Hiramatsu <mhiramat@...nel.org>

Powered by blists - more mailing lists