[L] Change in coreboot[main]: libpayload: Add x86_64 (64-bit) support

[Julius Werner (Code Review)]

Attention is currently required from: Arthur Heymans, Kapil Porwal, Subrata Banik.

Julius Werner 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 74:

(9 comments)

File payloads/libpayload/arch/x86/head_64.S:

https://review.coreboot.org/c/coreboot/+/81968/comment/6f9b2118_5ccf6411?usp... :
PS74, Line 38:  * Payload Entry Point Behavior with below code.
This table is okay but I think there should also be a more explicit warning that this code needs to run in both modes and certain instructions need to be avoided. How about:
```
WARNING: The payload is designed to be entered in either 32-bit protected or 64-bit
long mode, and the below code (between `_entry` and `jnz _init64`) must work correctly
in both modes. That means you should only use instructions that assemble to the same
binary representation in either mode. Any changes to this code should be tested for
both cases (entry from 32-bit and entry from 64-bit mode).
```

https://review.coreboot.org/c/coreboot/+/81968/comment/85a7afed_efe59ac7?usp... :
PS74, Line 41: LP_ARCH_X86_64
This column seems pointless.

https://review.coreboot.org/c/coreboot/+/81968/comment/17ae0d51_f8ffa7cf?usp... :
PS74, Line 121: 
What about `fninit`?

File payloads/libpayload/arch/x86/pt.S:

https://review.coreboot.org/c/coreboot/+/81968/comment/fd701428_9834f4f1?usp... :
PS74, Line 51: .global pde_table
I think these names are problematic now because you're using `pde_table` for the PDE table in the 2M paging case but for the PDPTE table in the 1GB paging case. Would probably make more sense to call these `main_page_table` and `extra_page_table` or something like that.

https://review.coreboot.org/c/coreboot/+/81968/comment/b8dfa223_9c56c36a?usp... :
PS74, Line 65: .type init_page_table, @function
Should be a similar warning here, I'd suggest:
```
WARNING: The `init_page_table` function is designed to be entered in either 32-bit
protected or 64-bit long mode and must work correctly in both modes. That means you
should only use instructions that assemble to the correct binary representation for
either mode.

We are building this with `.code64` to force the assembler to use the longer
representation of the `inc` instruction (`FF r/m32` instead of `4X`), because the
latter is only valid in 32-bit mode (would be a REX-prefix in 64-bit mode). When using
register-indirect addressing (e.g. `mov %eax, (%rsi, %rcx, 8)`), the source register
operands need to be specified in 64-bit notation (`%rsi` instead of `%esi`) to prevent
the assembler from generating an extra address size prefix (`67`) that means something
different in 32-bit mode. This is fine because `mov $..., %esi` will set the high 32-bits
of the `%rsi` register to 0 in 64-bit mode, so it doesn't matter whether we're using
to the 32-bit or 64-bit version of the register.

Any changes to this code should be tested for both cases (entry from 32-bit and
entry from 64-bit mode).
```

https://review.coreboot.org/c/coreboot/+/81968/comment/862f7741_9f5ab778?usp... :
PS74, Line 75: 	mov $(_PRES + _RW + _US + _A + _D), %eax
I still don't understand why you're not setting `_PS` here tbh. Am I misunderstanding how this works? Table 4-17 "Format of an IA-32e Page-Directory Entry that Maps a 2-MByte Page" says "7 (PS) |  Page size; must be 1 (otherwise, this entry references a page table; see Table 4-18)".

Did you test this code (commenting out the `jnz` above to force it to run) and confirm that it works this way?

https://review.coreboot.org/c/coreboot/+/81968/comment/269f5da9_c3723081?usp... :
PS74, Line 82: 	mov %ebx, 4(%rsi, %rcx, 8)
Actually, the same argument as below also applies here: we're only writing 2MB tables for the low 4GB of memory, so we know that EAX will never overflow. Can simplify this to
```
loop_2mb:
mov %eax, (%rsi, %rcx, 8)
mov $0, 4(%rsi, rcx, 8)
add $0x200000, %eax
inc %ecx
cmp %edi, %ecx
jb fill_pdpe
```

https://review.coreboot.org/c/coreboot/+/81968/comment/02987c87_9a45f6d4?usp... :
PS74, Line 104: 	cmp %edx, %eax
Sorry, I'm very confused about what you're doing here and what you're using EDX for (instead of EAX like with the other versions of this loop). What's the point of adding EDX to EAX and then comparing with EAX? You're not initializing EAX anywhere so it would still point to the end of the PDE table from the previous loop (and then you're adding more to it)?

In general, the point of this overflow check is just to see if we need to increment EBX because EAX overflowed over the 4GB limit. In this case, since we know that EAX just walks the PDE table and libpayload (including the entire PDE table in BSS) is linked below 4GB, we can actually skip that entire check because we know that it will never cross that boundary (and EBX will never need to be anything other than 0). So in fact I think we can simplify this entire thing to:
```
mov $4, %edi
mov $(_PRES + _RW + _US + _A + pde_table), %eax
mov $0, %ebx
mov $0, %ecx
mov $pdpe_table, %esi

fill_pdpe:
mov %eax, (%rsi, %rcx, 8)
mov $0, 4(%rsi, rcx, 8)
add $4096, %eax
inc %ecx
cmp %edi, %ecx
jb fill_pdpe
```
(I don't know if the linker supports a relocation for a construct like `mov $(_PRES + _RW + _US + _A + pde_table), %eax`. If not, then just do `mov $pde_table, %eax` and `add $(_PRES + _RW + _US + _A), %eax`.)

https://review.coreboot.org/c/coreboot/+/81968/comment/c886c8f4_f07e221c?usp... :
PS74, Line 116: 	jmp leave
nit: Easier to just do `ret` here?