From: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
To: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
<ysato@users.sourceforge.jp>, <dalias@libc.org>
Cc: <linux-sh@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<robh+dt@kernel.org>, <kernel@quicinc.com>
Subject: Re: [PATCH v4] sh: Restructure setup code to reserve memory regions earlier
Date: Wed, 17 Jul 2024 19:22:42 -0700 [thread overview]
Message-ID: <636943c1-6e32-4dd1-abdd-5a110e9aa07c@quicinc.com> (raw)
In-Reply-To: <831887db73d9eafc50940315ed44139107bd5f2a.camel@physik.fu-berlin.de>
On 7/16/2024 3:42 AM, John Paul Adrian Glaubitz wrote:
> Hi Oreoluwa,
>
> On Mon, 2024-07-15 at 17:12 -0700, Oreoluwa Babatunde wrote:
>> 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.
> Okay, I will wait with my pull request to Linus a few more days then.
>
> Thanks so much for being super patient with me. It took me way too long
> to test and review your patch, but I hope in the end we'll get the best
> possible version merged.
>
> Adrian
Hi Adrian,
Thanks for your feedback and for working with me on this.
I have uploaded a new version here:
https://lore.kernel.org/all/20240718021822.1545976-1-quic_obabatun@quicinc.com/
Please let me know if this properly addresses your comments.
Regards,
Oreoluwa
next prev parent reply other threads:[~2024-07-18 2:23 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-11 21:44 [PATCH v4] sh: Restructure setup code to reserve memory regions earlier Oreoluwa Babatunde
2024-07-12 13:12 ` John Paul Adrian Glaubitz
2024-07-13 7:58 ` John Paul Adrian Glaubitz
2024-07-16 0:12 ` Oreoluwa Babatunde
2024-07-16 10:42 ` John Paul Adrian Glaubitz
2024-07-18 2:22 ` Oreoluwa Babatunde [this message]
2024-07-23 7:57 ` John Paul Adrian Glaubitz
2024-07-23 16:59 ` Oreoluwa Babatunde
2024-08-17 7:14 ` John Paul Adrian Glaubitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=636943c1-6e32-4dd1-abdd-5a110e9aa07c@quicinc.com \
--to=quic_obabatun@quicinc.com \
--cc=dalias@libc.org \
--cc=glaubitz@physik.fu-berlin.de \
--cc=kernel@quicinc.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=ysato@users.sourceforge.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).