Date: Fri, 21 Jun 2024 14:02:18 +0800
From: yunhui cui <cuiyunhui@...edance.com>
To: Charlie Jenkins <charlie@...osinc.com>
Cc: paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu, 
	alexghiti@...osinc.com, akpm@...ux-foundation.org, bhe@...hat.com, 
	rppt@...nel.org, dawei.li@...ngroup.cn, jszhang@...nel.org, 
	namcao@...utronix.de, bjorn@...osinc.com, vishal.moola@...il.com, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH] RISC-V: cmdline: Add support for 'memmap' parameter

Hi Charlie，

On Fri, Jun 21, 2024 at 11:10 AM Charlie Jenkins <charlie@...osinc.com> wrote:
>
> On Fri, Jun 21, 2024 at 10:08:39AM +0800, yunhui cui wrote:
> > Hi Charlie,
> >
> > On Fri, Jun 21, 2024 at 9:03 AM Charlie Jenkins <charlie@...osinc.com> wrote:
> > >
> > > On Tue, Jun 18, 2024 at 08:08:42PM +0800, Yunhui Cui wrote:
> > > > Implement support for parsing 'memmap' kernel command line parameter.
> > > >
> > > > This patch covers parsing of the following two formats for 'memmap'
> > > > parameter values:
> > > >
> > > > - nn[KMG]@ss[KMG]
> > > > - nn[KMG]$ss[KMG]
> > > >
> > > > ([KMG] = K M or G (kilo, mega, giga))
> > > >
> > > > These two allowed formats for parameter value are already documented
> > > > in file kernel-parameters.txt in Documentation/admin-guide folder.
> > > > Some architectures already support them, but Mips did not prior to
> > >
> > > Copy-paste from a Mips patch? Should say riscv :)
> > >
> > > It looks like this code is duplicated from xtensa and is effectively the
> > > same as mips. Can this code be placed in a generic file so that the code
> > > can be shared between mips, riscv, and xtensa -- maybe a new config that
> > > gets selected by mips/riscv/xtensa?
> >
> > Yeah, that's actually what I was thinking. Which general file do you
> > think would be more suitable to put it in?
>
> I am not sure the best place to put it. What do you think about
> mm/memblock.c next to the "memblock" early param?

Is it inappropriate to put it in memblock? The implementation of mips
is different from that of xtensa, and early_mem is also distributed in
various archs, so we still put memmap in riscv/, and then I will
modify the commit log.
What do you think?

>
> >
> > > - Charlie
> > >
> > > > this patch.
> > > >
> > > > Excerpt from Documentation/admin-guide/kernel-parameters.txt:
> > > >
> > > > memmap=nn[KMG]@ss[KMG]
> > > > [KNL] Force usage of a specific region of memory.
> > > > Region of memory to be used is from ss to ss+nn.
> > > >
> > > > memmap=nn[KMG]$ss[KMG]
> > > > Mark specific memory as reserved.
> > > > Region of memory to be reserved is from ss to ss+nn.
> > > > Example: Exclude memory from 0x18690000-0x1869ffff
> > > > memmap=64K$0x18690000
> > > > or
> > > > memmap=0x10000$0x18690000
> > > >
> > > > There is no need to update this documentation file with respect to
> > > > this patch.
> > > >
> > > > Signed-off-by: Yunhui Cui <cuiyunhui@...edance.com>
> > > > ---
> > > >  arch/riscv/mm/init.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 50 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > > index e3405e4b99af..7be7ec3092ad 100644
> > > > --- a/arch/riscv/mm/init.c
> > > > +++ b/arch/riscv/mm/init.c
> > > > @@ -208,6 +208,56 @@ 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;
> > > > +
> > > > +     case 0:
> > > > +             memblock_reserve(mem_size, -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
> > > >
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@...ts.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> >
> > Thanks,
> > Yunhui

Thanks,
Yunhui

Powered by blists - more mailing lists