Attention is currently required from: Michał Żygowski, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59523 )
Change subject: nb/intel/sandybridge/romstage.c: Configure DPR and initialize TXT ......................................................................
Patch Set 3:
(4 comments)
File src/northbridge/intel/sandybridge/romstage.c:
https://review.coreboot.org/c/coreboot/+/59523/comment/1e9b79ca_3e7ebb26 PS2, Line 78: #if CONFIG(INTEL_TXT)
But then if INTEL_TXT is not selected in the configuration, the intel_txt_romstage_init will not hav […]
Please don't add weak symbols. Just look at what Haswell code does. Yes, it doesn't configure DPR, it relies on a patched MRC.bin doing it. I didn't think of programming and locking the DPR register before running MRC.
File src/northbridge/intel/sandybridge/romstage.c:
https://review.coreboot.org/c/coreboot/+/59523/comment/a47ddcde_321f6d87 PS3, Line 28: #if CONFIG(INTEL_TXT) Is this guard needed?
https://review.coreboot.org/c/coreboot/+/59523/comment/1c13da94_b4cc919b PS3, Line 35: /* 3 MiB should be enough */ I'd omit this comment.
https://review.coreboot.org/c/coreboot/+/59523/comment/5bfadf9b_f11d7990 PS3, Line 36: pci_write_config32(HOST_BRIDGE, DPR, dpr.raw); Where does dpr.top come from?