Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30855 )
Change subject: arch/x86: Align _start16bit with C_ENVIRONMENT_BOOBLOCK ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/30855/2/src/cpu/x86/16bit/entry16.inc File src/cpu/x86/16bit/entry16.inc:
https://review.coreboot.org/#/c/30855/2/src/cpu/x86/16bit/entry16.inc@32 PS2, Line 32: #if IS_ENABLED(CONFIG_C_ENVIRONMENT_BOOTBLOCK) || \
objdump -M i8086 -d build/cbfs/fallback/bootblock.elf […]
Ya, it's just reliant on the wrap it seems with starting at the beginning of the segment.
(gdb) p/x 0xfffffff3 + 0x000d $1 = 0x0 gdb) p/x 0xfffffff3 + 0xffff000d $2 = 0xffff0000
So yes, you are right in that we're explicitly taking advantage of that:
-------.byte 0xe9 -------.int _start16bit - ( . + 2 )
We aren't relying on the linker picking the correct relocation offset. We're just forcing the math to rollover.
I don't think it hurts to add a sanity check w.r.t. enforcing our assumptions. In the current form, that's C_ENV_BOOTBLOCK_SIZE <= 64KiB. But, yes, you are correct that it's not strictly tied to this CL. However, having that check in place can fix any potential future breakage. I was hoping it'd be small enough that you could put it in here or another patch.