diff mbox series

[2/3] binman: Prevent entries in a section from overlapping

Message ID 20211013023022.58829-3-samuel@sholland.org
State Deferred
Delegated to: Tom Rini
Headers show
Series sunxi: SPL FIT support for 32-bit sunxi SoCs | expand

Commit Message

Samuel Holland Oct. 13, 2021, 2:30 a.m. UTC
Currently, if the "offset" property is given for an entry, the section's
running offset is completely ignored. This causes entries to overlap if
the provided offset is less than the size of the entries earlier in the
section. Avoid the overlap by only using the provided offset when it is
greater than the running offset.

The motivation for this change is the rule used by SPL to find U-Boot on
sunxi boards: U-Boot starts 32 KiB after the start of SPL, unless SPL is
larger than 32 KiB, in which case U-Boot immediately follows SPL.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 tools/binman/entry.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Simon Glass Oct. 13, 2021, 6:06 p.m. UTC | #1
Hi Samuel,

On Tue, 12 Oct 2021 at 20:30, Samuel Holland <samuel@sholland.org> wrote:
>
> Currently, if the "offset" property is given for an entry, the section's
> running offset is completely ignored. This causes entries to overlap if
> the provided offset is less than the size of the entries earlier in the
> section. Avoid the overlap by only using the provided offset when it is
> greater than the running offset.
>
> The motivation for this change is the rule used by SPL to find U-Boot on
> sunxi boards: U-Boot starts 32 KiB after the start of SPL, unless SPL is
> larger than 32 KiB, in which case U-Boot immediately follows SPL.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>
>  tools/binman/entry.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index 70222718ea..61822eb5e4 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -404,7 +404,9 @@ class Entry(object):
>              if self.offset_unset:
>                  self.Raise('No offset set with offset-unset: should another '
>                             'entry provide this correct offset?')
> -            self.offset = tools.Align(offset, self.align)
> +        elif self.offset > offset:
> +            offset = self.offset
> +        self.offset = tools.Align(offset, self.align)
>          needed = self.pad_before + self.contents_size + self.pad_after
>          needed = tools.Align(needed, self.align_size)
>          size = self.size
> --
> 2.32.0
>

The behaviour needs to be deterministic, here. You have two cases, as
I understand it

- you want the offset to be set to the running offset, if the previous
section is >=32KB
- you want the offset to be set to the 'offset' property otherwise

I see a few options:

- Add a new 'sunxi-spl-pad' etype between SPL and U-Boot for the
padding. It can decide its own size
- Add a new 'min-size' property to entries, update the packer, use it
in the SPL node, set to 32KB

There are probably others. No need to worry about it if you are just
putting out ideas, but we do need a test for any new behaivour:

binman test -T

needs to pass with 100% code coverage.

Regards,
Simon
diff mbox series

Patch

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 70222718ea..61822eb5e4 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -404,7 +404,9 @@  class Entry(object):
             if self.offset_unset:
                 self.Raise('No offset set with offset-unset: should another '
                            'entry provide this correct offset?')
-            self.offset = tools.Align(offset, self.align)
+        elif self.offset > offset:
+            offset = self.offset
+        self.offset = tools.Align(offset, self.align)
         needed = self.pad_before + self.contents_size + self.pad_after
         needed = tools.Align(needed, self.align_size)
         size = self.size