Re: [PATCH v9 01/19] x86/boot: Place kernel_info at a fixed offset

From: ross . philipson
Date: Tue Jun 04 2024 - 16:29:49 EST


On 6/4/24 11:18 AM, Jarkko Sakkinen wrote:
On Fri May 31, 2024 at 4:03 AM EEST, Ross Philipson wrote:
From: Arvind Sankar <nivedita@xxxxxxxxxxxx>

There are use cases for storing the offset of a symbol in kernel_info.
For example, the trenchboot series [0] needs to store the offset of the
Measured Launch Environment header in kernel_info.

So either there are other use cases that you should enumerate, or just
be straight and state that this is done for Trenchboot.

The kernel_info concept came about because of the work we were doing on TrenchBoot but it was not done for TrenchBoot. It was a collaborative effort between the TrenchBoot team and H. Peter Anvin at Intel. He actually envisioned it being useful elsewhere. If you find the original commits for it (that went in stand-alone) from Daniel Kiper, there is a fair amount of detail what kernel_info is supposed to be and should be used for.


I believe latter is the case, and there is no reason to project further.
If it does not interfere kernel otherwise, it should be fine just by
that.

Also I believe that it is written as Trenchboot, without "series" ;-)
Think when writing commit message that it will some day be part of the
commit log, not a series flying in the air.

Sorry for the nitpicks but better to be punctual and that way also
transparent as possible, right?

No problem. We submit the patch sets to get feedback :)

Thanks for the feedback.



Since commit (note: commit ID from tip/master)

commit 527afc212231 ("x86/boot: Check that there are no run-time relocations")

run-time relocations are not allowed in the compressed kernel, so simply
using the symbol in kernel_info, as

.long symbol

will cause a linker error because this is not position-independent.

With kernel_info being a separate object file and in a different section
from startup_32, there is no way to calculate the offset of a symbol
from the start of the image in a position-independent way.

To enable such use cases, put kernel_info into its own section which is

"To allow Trenchboot to access the fields of kernel_info..."

Much more understandable.

placed at a predetermined offset (KERNEL_INFO_OFFSET) via the linker
script. This will allow calculating the symbol offset in a
position-independent way, by adding the offset from the start of
kernel_info to KERNEL_INFO_OFFSET.

Ensure that kernel_info is aligned, and use the SYM_DATA.* macros
instead of bare labels. This stores the size of the kernel_info
structure in the ELF symbol table.

Aligned to which boundary and short explanation why to that boundary,
i.e. state the obvious if you bring it up anyway here.

Just seems to be progressing pretty well so taking my eye glass and
looking into nitty gritty details...

So a lot of this is up in the air if you read the responses between us and Ard Biesheuvel. It would be nice to get rid of the part where kernel_info is forced to a fixed offset in the setup kernel.

Thanks
Ross


BR, Jarkko