9 comments:
Patch Set #17, Line 20: Remove the postcar stage.
Sorry, I wasn't clear. I just meant the wording. The postcar stage is […]
Oh, I see what you mean. postcar was removed from the execution flow of any of these processors, but not removed out of coreboot.
Patch Set #17, Line 27: they can be linked with their .data and .bss as normal
fwiw this is not specific to running a stage from dram, but can be done on all stages not running XI […]
Agree. Better wording for my intention was preram stages, not all stages. I'll describe it better.
Patch Set #15, Line 1162: # Ignore segments for .car.data or .earlyram.data while adding romstage.
any news on this? can this be closed?
I don't see anything asserting empty .data. If I missed it, or if it's in review I believe it can stay separate from this change.
However, given the current nature of this patch, the change here is no longer a requirement. We're keeping the .data section in romstage and there seems to be no harm in telling cbfs to strip a nonexistent .car.data.
File src/arch/x86/early_ram.ld:
/* Account for the size of .near_reset_vector regardless if it's used */
_NEAR_RESET_VECTOR = CONFIG_X86_RESET_VECTOR - 0x1000 + 0x10;
With the bootblock approach, I'm nowhere close to needing >64K. […]
Done
File src/arch/x86/early_ram.ld:
The other style option is /* and */ on the first and last line without preceding *s. i.e. […]
Done
Patch Set #17, Line 18: _PAD_FOR_ALIGNS = 0x20;
Agree, none of the other macros use ALIGN_COUNTER. The REGION macro also verifies the alignment. […]
Ah, you're mostly right 😊 Although the stack itself must be aligned, the region in memory may be unaligned. However, it looks like I can't remove all alignment here. Although the reset vector is on a 16-byte boundary, the subtraction below doesn't result in a nice address -- the FMAP_SIZE value below is 0xb6. So even if EARLYRAM_STACK is a don't-care, console and timestamps will subsequently fail their asserts of 4 and 8.
I'll use a more natural looking way to align it here and remove the alignment from memlayout.h.
Patch Set #17, Line 23: _PAD_FOR_ALIGNS
See description above.
Done
Patch Set #17, Line 30: #endif
We can also assert here that `. <= CONFIG_X86_RESET_VECTOR`.
Done
EARLYRAM_STACK(., _STACK_SIZE)
PRERAM_CBMEM_CONSOLE(., _CONSOLE_SIZE)
TIMESTAMP(., _TIMESTAMPS_SIZE)
#if !CONFIG(NO_FMAP_CACHE)
FMAP_CACHE(., FMAP_SIZE)
#endif
Can we add that in a follow-on change? You may know that the PSP will be able to run vboot before t […]
Ack.
Thinking a little more about this, the first implementation will run vboot on the PSP itself and it'll ship the workbuf to DRAM for us. We need to provide a target address for the PSP to use and give that to amdfwtool, ensuring it matches a region we declare here.
A vboot implementation I don't want to rule out is one that runs more like the traditional model, i.e. bootblock->verstage->romstage. In that case, a VBOOT2_WORK region anywhere we declare it should be fine. No need to coordinate with the PSP's amdfw.rom build.
BTW option 1 above will only be available on certain SKUs. Option 2 does not afford the ability to verify the memory init functionality (ABLs).
To view, visit change 35035. To unsubscribe, or for help writing mail filters, visit settings.