[External] Re: [PATCH v2] RISC-V: cmdline: Add support for 'memmap' parameter

[yunhui cui]

Hi Palmer,

On Sat, Sep 14, 2024 at 4:21 PM Palmer Dabbelt <palmer at dabbelt.com> wrote:
>
> On Mon, 22 Jul 2024 14:13:14 PDT (-0700), punit.agrawal at bytedance.com wrote:
> > Hi Palmer,
> >
> > Charlie Jenkins <charlie at rivosinc.com> writes:
> >
> >> On Mon, Jun 24, 2024 at 08:37:39PM +0800, Yunhui Cui wrote:
> >>> Add parsing of 'memmap' to use or reserve a specific region of memory.
> >>>
> >>> Implement the following memmap variants:
> >>> - memmap=nn[KMG]@ss[KMG]: force usage of a specific region of memory;
> >>> - memmap=nn[KMG]$ss[KMG]: mark specified memory as reserved;
> >>>
> >>> Signed-off-by: Yunhui Cui <cuiyunhui at bytedance.com>
> >>> ---
> >>>  arch/riscv/mm/init.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 46 insertions(+)
> >>>
> >>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> >>> index e3405e4b99af..8e1d93ae5cb2 100644
> >>> --- a/arch/riscv/mm/init.c
> >>> +++ b/arch/riscv/mm/init.c
> >>> @@ -208,6 +208,52 @@ static int __init early_mem(char *p)
> >>>  }
> >>>  early_param("mem", early_mem);
> >>>
> >>> +static void __init parse_memmap_one(char *p)
> >>> +{
> >>> +   char *oldp;
> >>> +   unsigned long start_at, mem_size;
> >>> +
> >>> +   if (!p)
> >>> +           return;
> >>> +
> >>> +   oldp = p;
> >>> +   mem_size = memparse(p, &p);
> >>> +   if (p == oldp)
> >>> +           return;
> >>> +
> >>> +   switch (*p) {
> >>> +   case '@':
> >>> +           start_at = memparse(p + 1, &p);
> >>> +           memblock_add(start_at, mem_size);
> >>> +           break;
> >>> +
> >>> +   case '$':
> >>> +           start_at = memparse(p + 1, &p);
> >>> +           memblock_reserve(start_at, mem_size);
> >>> +           break;
> >>> +
> >>> +   default:
> >>> +           pr_warn("Unrecognized memmap syntax: %s\n", p);
> >>> +           break;
> >>> +   }
> >>> +}
> >>> +
> >>> +static int __init parse_memmap_opt(char *str)
> >>> +{
> >>> +   while (str) {
> >>> +           char *k = strchr(str, ',');
> >>> +
> >>> +           if (k)
> >>> +                   *k++ = 0;
> >>> +
> >>> +           parse_memmap_one(str);
> >>> +           str = k;
> >>> +   }
> >>> +
> >>> +   return 0;
> >>> +}
> >>> +early_param("memmap", parse_memmap_opt);
> >>> +
> >>>  static void __init setup_bootmem(void)
> >>>  {
> >>>     phys_addr_t vmlinux_end = __pa_symbol(&_end);
> >>> --
> >>> 2.20.1
> >>>
> >>
> >> Reviewed-by: Charlie Jenkins <charlie at rivosinc.com>
> >
> > Another patch that looks good to get merged if there are no further
> > comments.
> >
> > Any chance this can be picked up for this cycle?
>
> Sorry for being slow here, but I don't understand the use case for this:
> we already get the memory map from the firmware, it seems like
> overriding that is just asking for issues.

Without modifying the firmware, it is more convenient to complete the
reservation of the memory block in this way.
This parameter is also mentioned in Documentation/admin - guide/kernel
- parameters.txt.

Thanks,
Yunhui