8 comments:
Patch Set #6, Line 87: reset vector in RAM instead of the traditional 0xfffffff0 location.
The name is just killing me now. […]
Name is suitable for the purpose it is used in rules.h. I would like to keep the window open for separate bootblock/verstage/romstage approach, as nobody wants to answer questions on the mailing list about picasso VBOOT build process.
ARMs run romstage (and bootblock and verstage) from SRAM, maybe this can be generalised even more and drop 'depends on ARCH_X86'.
File src/arch/x86/early_dram.ld:
Patch Set #6, Line 29: . += CONFIG_EARLYRAM_BSP_STACK_SIZE;
Do you really need to execute APs in romstage?
Patch Set #6, Line 58: ap_sipi_vector_in_rom = (_start16bit >> 12) & 0xff;
Maybe just set ap_sipi_vector_in_rom to 0 and comment as unused. The related assert in failover.ld will disappear with next release.
File src/arch/x86/include/arch/cpu.h:
Patch Set #6, Line 298: asmlinkage void soc_hybrid_romstage_entry(uint32_t bist, uint64_t early_tsc);
I don't see why bist and early_tsc should be forwarded to a function in soc/?
File src/arch/x86/reset_in_dram_crt0.S:
Patch Set #6, Line 37: jmp soc_reassign_stack /* expect returning jmp to zero_stacks */
> Can we use a different ABI to indicate return symbol? like another register's value such as ebp, e […]
AGESA/binaryPI C_ENVIRONMENT_BOOTBLOCK implementation would skip assembly_entry.S, like amd/stoneyridge does, for BSPs.
Maybe you should bring early_stack.S from CB:33759 into this commit.
Patch Set #6, Line 59: call soc_hybrid_romstage_entry
Why soc_ prefix? Can you design this from day one such that code that is not specific to silicon is not under soc/.
TBH this looks very much like booblock_c_entry_bist from every aspect now, the point where you enter C after in protected mode? While the ChromeOS device may not need it, I would prefer if you kept most of the separate bootblock/romstage separation as an option.
Patch Set #6, Line 61: /* This is here for linking purposes. */
Is this really needed?
Patch Set #6, Line 32: depends on RESET_VECTOR_IN_RAM
Do you need a static value for this?
I would prefer these grouped under single 'if RESET_VECTOR_IN_RAM'.
To view, visit change 35035. To unsubscribe, or for help writing mail filters, visit settings.