Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35993 )
Change subject: cpu/x86: Add a prog_run hook to set up caching of XIP stages ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35993/8/src/cpu/x86/mtrr/xip_cache.... File src/cpu/x86/mtrr/xip_cache.c:
https://review.coreboot.org/c/coreboot/+/35993/8/src/cpu/x86/mtrr/xip_cache.... PS8, Line 22: void platform_prog_run(struct prog *prog)
If this is being ran multiple times aren't we just accumulating MTRRs? Admittedly it's currently o […]
Working fine is a function of the ROM layout and cpu cache microarchitecture. Please don't assume all variations will continue to work when those parameters change.
Also, please add a comment somewhere how this is known to accumulate MTRRs when it's executed multiple times.
https://review.coreboot.org/c/coreboot/+/35993/8/src/cpu/x86/mtrr/xip_cache.... PS8, Line 33: if (cpu_info.x86 == 0xf) {
I'm not sure this a valid check. x86 is already checked for 0xf in get_fms(). […]
My point is that the x86 field is manipulated when it's == 0xf. Are you suggesting that after the manipulation the family will remain 0xf on netburst?
Either way, there needs to be a comment added describing what you are trying to accomplish.