Re: [PATCH v4] sh: Restructure setup code to reserve memory regions earlier

On 7/13/2024 12:58 AM, John Paul Adrian Glaubitz wrote:

Hi Adrian,

>> diff --git a/arch/sh/include/asm/setup.h b/arch/sh/include/asm/setup.h
>> index 84bb23a771f3..f8b814fb1c7f 100644
>> --- a/arch/sh/include/asm/setup.h
>> +++ b/arch/sh/include/asm/setup.h
>> @@ -19,7 +19,6 @@
>>  #define COMMAND_LINE ((char *) (PARAM+0x100))
>>  
>>  void sh_mv_setup(void);
>> -void check_for_initrd(void);
>>  void per_cpu_trap_init(void);
>>  void sh_fdt_init(phys_addr_t dt_phys);
>>  
>> diff --git a/arch/sh/kernel/setup.c b/arch/sh/kernel/setup.c
>> index 620e5cf8ae1e..8477491f4ffd 100644
>> --- a/arch/sh/kernel/setup.c
>> +++ b/arch/sh/kernel/setup.c
>> @@ -35,6 +35,7 @@
>>  #include <asm/io.h>
>>  #include <asm/page.h>
>>  #include <asm/elf.h>
>> +#include <asm/kexec.h>
>>  #include <asm/sections.h>
>>  #include <asm/irq.h>
>>  #include <asm/setup.h>
>> @@ -114,7 +115,7 @@ static int __init early_parse_mem(char *p)
>>  }
>>  early_param("mem", early_parse_mem);
>>  
>> -void __init check_for_initrd(void)
>> +static void __init check_for_initrd(void)
>>  {
>>  #ifdef CONFIG_BLK_DEV_INITRD
>>  	unsigned long start, end;
>> @@ -172,6 +173,42 @@ void __init check_for_initrd(void)
>>  #endif
>>  }
> Making check_for_initrd() static seems like an unrelated change to me or am
> I missing something? If yes, it should go into a separate patch.
ack.
>> +static void __init early_reserve_mem(void)
>> +{
>> +	unsigned long start_pfn;
>> +	u32 zero_base = (u32)__MEMORY_START + (u32)PHYSICAL_OFFSET;
>> +	u32 start = zero_base + (u32)CONFIG_ZERO_PAGE_OFFSET;
>> +
>> +	/*
>> +	 * Partially used pages are not usable - thus
>> +	 * we are rounding upwards:
>> +	 */
>> +	start_pfn = PFN_UP(__pa(_end));
>> +
>> +	/*
>> +	 * Reserve the kernel text and Reserve the bootmem bitmap. We do
>> +	 * this in two steps (first step was init_bootmem()), because
>> +	 * this catches the (definitely buggy) case of us accidentally
>> +	 * initializing the bootmem allocator with an invalid RAM area.
>> +	 */
>> +	memblock_reserve(start, (PFN_PHYS(start_pfn) + PAGE_SIZE - 1) - start);
>> +
>> +	/*
>> +	 * Reserve physical pages below CONFIG_ZERO_PAGE_OFFSET.
>> +	 */
>> +	if (CONFIG_ZERO_PAGE_OFFSET != 0)
>> +		memblock_reserve(zero_base, CONFIG_ZERO_PAGE_OFFSET);
>> +
>> +	/*
>> +	 * Handle additional early reservations
>> +	 */
>> +	check_for_initrd();
>> +	reserve_crashkernel();
>> +
>> +	if (sh_mv.mv_mem_reserve)
>> +		sh_mv.mv_mem_reserve();
>> +}
>> +
>>  #ifndef CONFIG_GENERIC_CALIBRATE_DELAY
>>  void calibrate_delay(void)
>>  {
> I'm not really happy with moving early_reserve_mem() from mm/init.c to
> kernel/setup.c. Can't we just leave it where it is while still keeping
> the changes to paging_init()?
ack.
>
>> @@ -319,9 +356,14 @@ void __init setup_arch(char **cmdline_p)
>>  
>>  	sh_mv_setup();
>>  
>> +	sh_mv.mv_mem_init();
>> +
>>  	/* Let earlyprintk output early console messages */
>>  	sh_early_platform_driver_probe("earlyprintk", 1, 1);
>>  
>> +	/* set aside reserved memory regions */
>> +	early_reserve_mem();
>> +
>>  #ifdef CONFIG_OF_EARLY_FLATTREE
>>  #ifdef CONFIG_USE_BUILTIN_DTB
>>  	unflatten_and_copy_device_tree();

I'll make adjustments based on your comments and
resend another version.

Thanks,
Oreoluwa