Attention is currently required from: Arthur Heymans, Julius Werner, Kapil Porwal.
Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/81968?usp=email )
Change subject: libpayload: Add x86_64 (64-bit) support ......................................................................
Patch Set 69:
(3 comments)
File payloads/libpayload/arch/x86/head_64.S:
https://review.coreboot.org/c/coreboot/+/81968/comment/d31ac93e_233e9a16?usp... : PS64, Line 100: movq %rax, %cr3
Which section are you quoting? I'm looking at 4.10.4.1 "Operations that Invalidate TLBs and Paging-Structure Caches":
looking further, I found the double write into cr3 was a W/A for the specific microarchitecture which later fixed with ucode update hence, I believe we don't need to worry about double write.
``` movq %rax, %cr3 movq %cr3, %rax /* Invalidate previously used TLB */ movq %rax, %cr3 ```
difficult to say as I don't have any HW debugger attached to see the cr3 value in between while invalidating.
https://review.coreboot.org/c/coreboot/+/81968/comment/bca7ea87_ae911d65?usp... : PS64, Line 118:
Remember that this payload can be booted from either 32-bit or 64-bit coreboot, so if 64-bit coreboot does something we cannot rely on that. In general, this handoff API seems to be written such that we should try to not rely on stuff as much as possible (e.g. a non-coreboot firmware could try to boot this through the multiboot API). So if 32-bit libpayload used to turn these on explicitly, we should probably continue to do that. (I don't think we need the CPUID checks if we're sure that there are no 64-bit CPUs that don't support those features, though, we can just turn them on unconditionally.)
notes!
File payloads/libpayload/include/x86/arch/exception.h:
https://review.coreboot.org/c/coreboot/+/81968/comment/0a444e03_d6a93eb9?usp... : PS64, Line 74: #else
Sorry, no, you were absolutely right earlier, I didn't pay close enough attention to the order. […]
Acknowledged
-
Subrata Banik (Code Review)