Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Shuo Liu, Tim Chu.
Patrick Rudolph has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85806?usp=email )
Change subject: cpu/x86/64bit: Install extended page tables in BSS ......................................................................
Patch Set 4:
(6 comments)
File src/cpu/x86/64bit/mmu.c:
https://review.coreboot.org/c/coreboot/+/85806/comment/735a6aae_4acae07b?usp... : PS3, Line 12: #define _PRES (1ULL << 0)
Can't you use the BIT macro?
Done
https://review.coreboot.org/c/coreboot/+/85806/comment/4911fe13_a46d9706?usp... : PS3, Line 49: * when 1GB PT aren't supported it maps 40 bits of the address space (512GiB).
IMO the x86_64 by default boot with 4-level paging and here the 40bit VA is mainly for consideration […]
Yes, it "only" increases BSS by 2.1MiB. Older platforms can access up to 40bit VA as access to 48bit VA would make pagetable too big (512MiB of page tables?) and those older platforms do likely not have such high amount of DRAM/MMIO anyways.
https://review.coreboot.org/c/coreboot/+/85806/comment/85655878_a17a9793?usp... : PS3, Line 64: int
size_t
Done
https://review.coreboot.org/c/coreboot/+/85806/comment/0981b97b_dd8ed45e?usp... : PS3, Line 59: if (cpu_supports_1gb_hugepage()) { : const uintptr_t max_addr = 1ULL * GiB * 512ULL * 512ULL; : const uint64_t incr = 1ULL * GiB; : : /* Build P4MLE */ : for (int i = 0; i < 512; i++) : p4mle[i] = _GEN_DIR(&pdpt[i * 512]); : : /* Build PDPT */ : uint64_t *page_table_ptr = pdpt; : for (uintptr_t addr = 0; addr < max_addr; addr += incr, page_table_ptr++) : *page_table_ptr = _GEN_PAGE(addr); : } else { : const uintptr_t max_addr = 2ULL * MiB * 512ULL * 512ULL; : const uint64_t incr = 2ULL * MiB; : : /* Build P4MLE */ : p4mle[0] = _GEN_DIR(&pdpt[0]); : : /* Build PDPT */ : for (int i = 0; i < 512; i++) : pdpt[i] = _GEN_DIR(&pdt[i * 512]); : : /* Build PDT */ : uint64_t *page_table_ptr = pdt; : for (uintptr_t addr = 0; addr < max_addr; addr += incr, page_table_ptr++) : *page_table_ptr = _GEN_PAGE(addr); : } :
I don't see that. Please explain how it can be improved without reducing readability.
Done
https://review.coreboot.org/c/coreboot/+/85806/comment/0684ce37_c365ff41?usp... : PS3, Line 157: */
It would be helpful if we could have comments here as well, e.g. […]
Done
https://review.coreboot.org/c/coreboot/+/85806/comment/2ad36fd6_421bb751?usp... : PS3, Line 160: /* Using 512 4K pages limits the usable address space */
Do we need to make sure the CPU is not working under 5-level paging? (though coreboot never enables […]
Done