Re: [PATCH] x86/e820: update code comment about e820_table_kexec

[Dave Young]

On Sat, 14 Sept 2024 at 19:20, Dave Young <dyoung@xxxxxxxxxx> wrote:
>
> The setup_data ranges are not reserved for kexec any more after
> commit fc7f27cda843 ("x86/kexec: Do not update E820 kexec table
> for setup_data"), so update the code comment here.
>
> Signed-off-by: Dave Young <dyoung@xxxxxxxxxx>
> ---
>  arch/x86/kernel/e820.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> Index: linux-x86/arch/x86/kernel/e820.c
> ===================================================================
> --- linux-x86.orig/arch/x86/kernel/e820.c       2024-09-14 10:39:57.423551301 +0800
> +++ linux-x86/arch/x86/kernel/e820.c    2024-09-14 18:56:30.158316496 +0800
> @@ -36,10 +36,8 @@
>   *
>   * - 'e820_table_kexec': a slightly modified (by the kernel) firmware version
>   *   passed to us by the bootloader - the major difference between
> - *   e820_table_firmware[] and this one is that, the latter marks the setup_data
> - *   list created by the EFI boot stub as reserved, so that kexec can reuse the
> - *   setup_data information in the second kernel. Besides, e820_table_kexec[]
> - *   might also be modified by the kexec itself to fake a mptable.
> + *   e820_table_firmware[] and this one is that e820_table_kexec[]
> + *   might be modified by the kexec itself to fake a mptable.
>   *   We use this to:
>   *
>   *       - kexec, which is a bootloader in disguise, uses the original E820
>

BTW,  the conversation below drived me to read the e820 code:
https://lore.kernel.org/all/CAMj1kXG1hbiafKRyC5qM1Vj5X7x-dmLndqqo2AYnHMRxDz-80w@xxxxxxxxxxxxxx/T/#u

It could be better to clean up the e820 tables, anyway the comment fix
in this patch itself is good for now.

Basically e820_table_firmware is used by kexec-tools kexec_load
implementation,  e820_table_kexec is used by kexec_file_load code to
pass to the 2nd kernel in boot params.

The e820_table_firmware is said to not be modified by the kernel in
the code comment, but this is not true, at least the sev code updates
the table.    The hibernate code generates crc32 checksum and verifies
it, but since AFAIK e820 table update only happens in boot phase, it
will be stable on runtime.  So we can just use e820_table_firmware for
kexec use and drop the e820_table_kexec. With the change the
kexec_file_load and kexec_load see the same memory ranges.

Otherwise I thought we can use just one e820 table, dropping
e820_table_firmware and e820_table_kexec, but then there will be
fragments and memory waste  due to the setup_data ranges are reserved
and updated in e820_table, so the e820_table_firmware being still be
used for kexec makes sense.

Anyway I need to think more about it,  please let me know if you have
any concerns.

Thanks
Dave