Hi Tom,
On Fri, 21 Jun 2024 at 16:12, Tom Rini trini@konsulko.com wrote:
On Fri, Jun 21, 2024 at 01:38:07PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 21 Jun 2024 at 13:19, Tom Rini trini@konsulko.com wrote:
On Fri, Jun 21, 2024 at 11:55:42AM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 21 Jun 2024 at 10:05, Tom Rini trini@konsulko.com wrote:
[snip]
Yes, I very much do not like guessing about 3 numbers instead of guessing about 1 number and using the standard mechanism we already have. Please use BOARD_SIZE_LIMIT as this is the standard mechanism to enforce size limits on U-Boot itself.
If it were that easy I would have sent a patch :-)
Here is the map for this board:
ImagePos Offset Size Name 00000000 00000000 00800000 rom ff800000 ff800000 00001000 intel-descriptor ff801000 ff801000 001ff000 intel-me ffef0000 ffef0000 000999f0 u-boot-with-ucode-ptr fff899f0 fff899f0 00005554 u-boot-dtb-with-ucode fff8ef50 fff8ef50 00000000 u-boot-ucode fff8ef50 fff8ef50 00000571 fdtmap fff90000 fff90000 00010000 intel-vga fffa0000 fffa0000 0002fc94 intel-mrc fffcfc94 fffcfc94 00000000 private-files fffff800 fffff800 00000070 x86-start16 fffffff0 fffffff0 00000005 x86-reset16 fffffff8 fffffff8 00000008 image-header
What limit should I set on what?
Is this a trick question? $ printf %d\n $(( 0xfff90000 - 0xffef0000)) 655360
Of course since we're less than that today, you can reduce it by whatever other magic numbers I'm not seeing but are part of your assumed sizes.
That limit is on u-boot-nodtb.bin. Even with a size (for that file) of 634816 it doesn't fit. I need to calculate a size based on the size of the dtb and the microcode...which of course can change.
Yes, and you're able to assume some size for them, which is what you put in the dts file?
I just put in a limit for the blobs, whose sizes are known.
- the U-Boot is the thing you are wanting to limit
- the dtb has microcode added
- the ucode is empty in this case
- the fdtmap is variable in size
So this all seems a bit backwards. The actual limit is that (u-boot-with-ucode-ptr + u-boot-dtb-with-ucode + u-boot-ucode + fdtmap) fits in the space available. Note that some boards don't have intel-vga or intel-mrc.
With the other patch I sent I can have a sensible limit for all x86 boards.
Did you miss the comments above?
No, I saw them. They're similar constraints to other systems.
And you can set the same sensible limit with the existing mechanism with the bonus of it not making x86 different from the rest?
I understand that it is possible to set a limit for u-boot-nodtb.bin but that is not accurate nor sufficient in the presence of blobs. The solution belongs in Binman.
Your series puts reasonable estimates on the size of the blobs and then will give a failure such as: binman: Node '/binman/rom/intel-vga': Offset 0xfff90000 (4294508544) overlaps with previous entry '/binman/rom/fdtmap' ending at 0xfff902e1 (4294509281) Which is not as nice as (I just threw in a limit): u-boot-nodtb.bin exceeds file size limit: limit: 0x927c0 bytes actual: 0x9a810 bytes excess: 0x8050 bytes make[1]: *** [/home/trini/work/u-boot/u-boot/Makefile:1359: u-boot-nodtb.bin] Error 1
And tells us how much we need to get back size wise. Aside from when using the actual blobs (and in which case a real error will be shown when trying to use them), it's always about making an estimate on the part of the system that we control.
Thanks for digging into this.
I wasn't aware that the limit was just an estimate. Since it produces a build error it seems to be enforced as a hard requirement. But I can set a limit and we'll see how things go.
I still like the binman block-size thing though. The error message binman provides actually tells you what to do (reduce the space of the things that are overflowing into intel-vga). So I'd like to add that too.
Regards, Simon