Hi Steve, On Thu, 16 Jul 2020 10:27:03 +0900 Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote: > Hi Steve, > > On Wed, 15 Jul 2020 20:02:12 -0400 > Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > > On Thu, 16 Jul 2020 07:38:43 +0900 > > Masami Hiramatsu <mhiramat@xxxxxxxxxx> 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. What would you think? I guess if we have other types of data appended to the initrd as similar to the bootconfig, I think we should add the multiple section support. But only for the bootconfig, we can just update the bootconfig data as I suggested, since it keeps the code simple. It might be a chiken-egg problem... Thank you, -- Masami Hiramatsu <mhiramat@xxxxxxxxxx>
