Re: [PATCH 3/5] x86/boot: Remove hardcoded boot_param constants

[Tom Lendacky]

On 7/23/26 22:02, Brian Gerst wrote:
> Use generated constants or sizeof() instead of hardcoded values.
> 
> No functional change.
> 
> Signed-off-by: Brian Gerst <brgerst@xxxxxxxxx>
> ---
>  arch/x86/include/asm/setup.h            | 3 ---
>  arch/x86/kernel/asm-offsets.c           | 2 ++
>  arch/x86/kernel/head_32.S               | 4 ++--
>  drivers/firmware/efi/libstub/x86-stub.c | 6 +++---
>  4 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index 5534f4aaf73b..d23f4272d930 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -21,11 +21,8 @@
>  
>  #endif /* __i386__ */
>  
> -#define PARAM_SIZE 4096		/* sizeof(struct boot_params) */
> -
>  #define OLD_CL_MAGIC		0xA33F
>  #define OLD_CL_ADDRESS		0x020	/* Relative to real mode data */
> -#define NEW_CL_POINTER		0x228	/* Relative to real mode data */
>  
>  #ifndef __ASSEMBLER__
>  #include <linux/cache.h>
> diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
> index 081816888f7a..0c49bb50188d 100644
> --- a/arch/x86/kernel/asm-offsets.c
> +++ b/arch/x86/kernel/asm-offsets.c
> @@ -105,6 +105,8 @@ static void __used common(void)
>  	OFFSET(BP_kernel_alignment, boot_params, hdr.kernel_alignment);
>  	OFFSET(BP_init_size, boot_params, hdr.init_size);
>  	OFFSET(BP_pref_address, boot_params, hdr.pref_address);
> +	OFFSET(BP_cmd_line_ptr, boot_params, hdr.cmd_line_ptr);
> +	DEFINE(SIZEOF_boot_params, sizeof(struct boot_params));
>  
>  	BLANK();
>  	DEFINE(PTREGS_SIZE, sizeof(struct pt_regs));
> diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
> index 5171cb746444..1f66ccc36fe8 100644
> --- a/arch/x86/kernel/head_32.S
> +++ b/arch/x86/kernel/head_32.S
> @@ -96,10 +96,10 @@ SYM_CODE_START(startup_32)
>   * page tables.
>   */
>  	movl $pa(boot_params),%edi
> -	movl $(PARAM_SIZE/4),%ecx
> +	movl $(SIZEOF_boot_params/4),%ecx
>  	cld
>  	rep movsl
> -	movl pa(boot_params) + NEW_CL_POINTER,%esi
> +	movl pa(boot_params) + BP_cmd_line_ptr,%esi
>  	andl %esi,%esi
>  	jz 1f			# No command line
>  	movl $pa(boot_command_line),%edi
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index cef32e2c82d8..c3522d417344 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -520,11 +520,11 @@ static efi_status_t efi_allocate_bootparams(efi_handle_t handle,
>  		return status;
>  	}
>  
> -	status = efi_allocate_pages(PARAM_SIZE, &alloc, ULONG_MAX);
> +	status = efi_allocate_pages(sizeof(struct boot_params), &alloc, ULONG_MAX);

Why not sizeof(*boot_params) here and each place below instead of
hardcoding the struct?

Thanks,
Tom

>  	if (status != EFI_SUCCESS)
>  		return status;
>  
> -	boot_params = memset((void *)alloc, 0x0, PARAM_SIZE);
> +	boot_params = memset((void *)alloc, 0x0, sizeof(struct boot_params));
>  	hdr	    = &boot_params->hdr;
>  
>  	/* Assign the setup_header fields that the kernel actually cares about */
> @@ -537,7 +537,7 @@ static efi_status_t efi_allocate_bootparams(efi_handle_t handle,
>  	/* Convert unicode cmdline to ascii */
>  	cmdline_ptr = efi_convert_cmdline(image);
>  	if (!cmdline_ptr) {
> -		efi_free(PARAM_SIZE, alloc);
> +		efi_free(sizeof(struct boot_params), alloc);
>  		return EFI_OUT_OF_RESOURCES;
>  	}
>