You already have several TODOs left behind like bootblock-y += cache_as_ram.S
Not in this patch.
MD: We've started capturing ideas for ways to streamline the bootflow once the system is fully functional, and ultimately I'd hope to pull this particular change back out once it's no longer useful.
The TODOs are indicators of what will require some pretty substantial work, and the intent to do so. I don't mind rewording it, however "todo" is a fairly common search term.
To include car.ld for this concept seems invalid to me. Also, we have ENV_CACHE_AS_RAM that should be adjusted accordingly and you should have CACHE_AS_RAM=n in your .config.
In my opinion, there are many things that are less than ideal with this initial approach. coreboot makes lots of assumptions for how an x86 behaves and they don't fit well with how Family 17h comes out of reset. When challenged with both porting new technology (which intends to rely on reference code that's not yet functional) and into a codebase that has no established paradigm for how it operates...
That's how we arrived at this approach, i.e. to make it functional first but with absolute minimal disruption to the non-amd directories. Otherwise, more stuff breaks than you can imagine by only looking at these patches. Once it's fully functional, then the changes required to make picasso a good port will be more understandable.
The alternative was to modify coreboot first before pushing picasso. However that seems like a much more difficult argument for a unique processor where no reviewers would be allowed to know how it works. I don't think people would be willing to consider those types of x86 patches without understanding the premise.
Last time someone approved this from AMD contractors, it took 5 years or more to have them addressed. And it was not AMD contractor who sorted it out.
I try not to get upset over things I can't change.
2 comments:
File src/arch/x86/assembly_entry.S:
Patch Set #6, Line 36: _romstage_in_ram_continue:
Comment says it's a jump. […]
It's because the preprocessor removes _start from this file.
Patch Set #6, Line 49: /* clear CAR_GLOBAL area as it is not shared */
None of this CAR is really correct when you already have DRAM...
Completely agree. See non-inline comments.
To view, visit change 32414. To unsubscribe, or for help writing mail filters, visit settings.