Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32375 )
Change subject: postcar_loader: Use size_t ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/32375/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/32375/13//COMMIT_MSG@9 PS13, Line 9: To make it easier to pop arguments from stack use size_t instead of int.
Yes. postcar is being ran from x86-64 mode as well as the stage that is pushing values. This is due to rmodules nature, which can be only loaded from a stage that has the same arch. If that's no longer true the postcar frame will be broken.
This is an implicit assumption. And the MSRs just happen to work because msrs are read and written as edx:eax while being pushed as two 64-bit values. I do think it's helpful to comment the assumptions and explicitly handle the various types. That doesn't exist in this patch where the code was previously indicating type size (uint32_t) it's now not w/ no explanation.
Right now all values are poped from stack, such that the consumer will always do "size_t" reads. We could pack msrs into one value, but I don't see the benefit.
It would be explicit in the handling of different types with source loader and target stage ISA.