Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34791 )
Change subject: soc/intel/cannonlake: Speed up postcar loading using intermediate caching ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34791/5/src/soc/intel/cannonlake/ro... File src/soc/intel/cannonlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/34791/5/src/soc/intel/cannonlake/ro... PS5, Line 142: set_var_mtrr(mtrr, base, size, MTRR_TYPE_WRPROT);
Yes, I can see that newer Intel code uses a similar approach. Trying to explain why code is correct by just referencing it doesn't work.
The question is WHY does it use to work, not which code also does it.
We are already in NEM mode hence those mechanism is different than the one you have mentioned above
1. NEM Init 2. Configure DataStack region as WB memory type using a single variable MTRR 3. Configure BIOS Code region as WP memory type using a variable MTRR
Notes for Use of Cache for Stack and Code 1. Use of INVD will result in loss of DataStack data 2. Use of WBINVD or CLFLUSH is not recommended.
The following code does use CR0.CD=1: Linux kernel: arch/x86/kernel/cpu/mtrr/mtrr.c coreboot ramstage MTRR code: src/cpu/x86/mtrr/mtrr.c coreboot intel car code: soc/intel/common/block/cpu/car/cache_as_ram.S
If it would be that easy to set up MTRRs from within CAR, why wasn't it done in 7f8afe063139f6fc7076a3e4edf6093a953792dc ?
For my understanding the postcar frame needs to be filled with MTRRs, because you can only updated them after CAR has been disabled.
yes, we are disabling CAR and updating all required MTRR filled by postcar frame inside postcar and enabling MTRR back & enabling cache.