linux-kernel - Re: [PATCH 3/4] x86/boot: split parsing of boot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z1GrPjyFQvhHYIIm@gmail.com>
Date: Thu, 5 Dec 2024 14:31:42 +0100
From: Ingo Molnar <mingo@...nel.org>
To: Mike Rapoport <rppt@...nel.org>
Cc: x86@...nel.org, Andy Lutomirski <luto@...nel.org>,
	Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...hat.com>,
	Ning Sun <ning.sun@...el.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
	tboot-devel@...ts.sourceforge.net
Subject: Re: [PATCH 3/4] x86/boot: split parsing of boot_params into a helper
 function


* Mike Rapoport <rppt@...nel.org> wrote:

> From: "Mike Rapoport (Microsoft)" <rppt@...nel.org>
> 
> Makes setup_arch a bit easier to comprehend.
> 
> No functional changes.
> 
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
> ---
>  arch/x86/kernel/setup.c | 72 +++++++++++++++++++++++------------------
>  1 file changed, 41 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index d299fe5bda25..d4bb9a2e8f15 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -426,6 +426,42 @@ static void __init parse_setup_data(void)
>  	}
>  }
>  
> +static void __init parse_boot_params(void)
> +{
> +	ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
> +	screen_info = boot_params.screen_info;
> +	edid_info = boot_params.edid_info;
> +#ifdef CONFIG_X86_32
> +	apm_info.bios = boot_params.apm_bios_info;
> +	ist_info = boot_params.ist_info;
> +#endif
> +	saved_video_mode = boot_params.hdr.vid_mode;
> +	bootloader_type = boot_params.hdr.type_of_loader;
> +	if ((bootloader_type >> 4) == 0xe) {
> +		bootloader_type &= 0xf;
> +		bootloader_type |= (boot_params.hdr.ext_loader_type+0x10) << 4;
> +	}
> +	bootloader_version  = bootloader_type & 0xf;
> +	bootloader_version |= boot_params.hdr.ext_loader_ver << 4;
> +
> +#ifdef CONFIG_BLK_DEV_RAM
> +	rd_image_start = boot_params.hdr.ram_size & RAMDISK_IMAGE_START_MASK;
> +#endif
> +#ifdef CONFIG_EFI
> +	if (!strncmp((char *)&boot_params.efi_info.efi_loader_signature,
> +		     EFI32_LOADER_SIGNATURE, 4)) {
> +		set_bit(EFI_BOOT, &efi.flags);
> +	} else if (!strncmp((char *)&boot_params.efi_info.efi_loader_signature,
> +		     EFI64_LOADER_SIGNATURE, 4)) {
> +		set_bit(EFI_BOOT, &efi.flags);
> +		set_bit(EFI_64BIT, &efi.flags);
> +	}
> +#endif
> +
> +	if (!boot_params.hdr.root_flags)
> +		root_mountflags &= ~MS_RDONLY;
> +}
> +
>  static void __init memblock_x86_reserve_range_setup_data(void)
>  {
>  	struct setup_indirect *indirect;
> @@ -803,35 +839,11 @@ void __init setup_arch(char **cmdline_p)
>  
>  	setup_olpc_ofw_pgd();
>  
> -	ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
> -	screen_info = boot_params.screen_info;
> -	edid_info = boot_params.edid_info;
> -#ifdef CONFIG_X86_32
> -	apm_info.bios = boot_params.apm_bios_info;
> -	ist_info = boot_params.ist_info;
> -#endif
> -	saved_video_mode = boot_params.hdr.vid_mode;
> -	bootloader_type = boot_params.hdr.type_of_loader;
> -	if ((bootloader_type >> 4) == 0xe) {
> -		bootloader_type &= 0xf;
> -		bootloader_type |= (boot_params.hdr.ext_loader_type+0x10) << 4;
> -	}
> -	bootloader_version  = bootloader_type & 0xf;
> -	bootloader_version |= boot_params.hdr.ext_loader_ver << 4;
> -
> -#ifdef CONFIG_BLK_DEV_RAM
> -	rd_image_start = boot_params.hdr.ram_size & RAMDISK_IMAGE_START_MASK;
> -#endif
> -#ifdef CONFIG_EFI
> -	if (!strncmp((char *)&boot_params.efi_info.efi_loader_signature,
> -		     EFI32_LOADER_SIGNATURE, 4)) {
> -		set_bit(EFI_BOOT, &efi.flags);
> -	} else if (!strncmp((char *)&boot_params.efi_info.efi_loader_signature,
> -		     EFI64_LOADER_SIGNATURE, 4)) {
> -		set_bit(EFI_BOOT, &efi.flags);
> -		set_bit(EFI_64BIT, &efi.flags);
> -	}
> -#endif
> +	/*
> +	 * Translate the fields of struct boot_param into global variables

s/struct boot_param
 /'struct boot_param'

> +	 * represting these parameters.

Typo...

> +	 */
> +	parse_boot_params();

Also, why not move this explanatory comment to the definition of the new 
helper function?

Thanks,

	Ingo