| Message ID | 20240618120842.15159-1-cuiyunhui@bytedance.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RISC-V: cmdline: Add support for 'memmap' parameter | expand |
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? - 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@bytedance.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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Charlie, On Fri, Jun 21, 2024 at 9:03 AM Charlie Jenkins <charlie@rivosinc.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? > - 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@bytedance.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@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv Thanks, Yunhui
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@rivosinc.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? > > > - 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@bytedance.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@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > Thanks, > Yunhui
Hi Charlie， On Fri, Jun 21, 2024 at 11:10 AM Charlie Jenkins <charlie@rivosinc.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@rivosinc.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@bytedance.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@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > Thanks, > > Yunhui Thanks, Yunhui
On Fri, Jun 21, 2024 at 02:02:18PM +0800, yunhui cui wrote: > Hi Charlie， > > On Fri, Jun 21, 2024 at 11:10 AM Charlie Jenkins <charlie@rivosinc.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@rivosinc.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? The mips implementation is very close to being the same, but I am not sure if the differences would prevent standardization. xtensa and now riscv would have identical implementations though so a generic memmap implementation could be only applied to those two archs. The "mem" early param is also scattered across archs as you point out, but that looks more fragmented in how the different architectures have implemented it. I will copy Mike Rapoport to see if he has any comments since he is the maintainer of memblock and memory management initialization. > > > > > > > > > > - 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@bytedance.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@lists.infradead.org > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > Thanks, > > > Yunhui > > Thanks, > Yunhui
On Thu, Jun 20, 2024 at 11:39:41PM -0700, Charlie Jenkins wrote: > On Fri, Jun 21, 2024 at 02:02:18PM +0800, yunhui cui wrote: > > Hi Charlie， > > > > On Fri, Jun 21, 2024 at 11:10 AM Charlie Jenkins <charlie@rivosinc.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@rivosinc.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 I believe you should add RISCV to the list of supported architectures for these options. > > > > > 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? > > The mips implementation is very close to being the same, but I am not > sure if the differences would prevent standardization. xtensa and now > riscv would have identical implementations though so a generic memmap > implementation could be only applied to those two archs. The memmap= is generally a hack to workaround issues with how firmware describes memory to the kernel so in a way it belongs to arch/ code. mips and xtensa already have different views on how this should be treated, not mentioning x86 that handles memmap= on e820 level rather than with memblock. > > > > > > 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); This will add a region to memblock.reserved, but if there is no memory there this information won't be available after boot, e.g. there won't be struct pages with PG_Reserved for this region. Is this your intention? > > > > > > + break; > > > > > > + > > > > > > + case 0: > > > > > > + memblock_reserve(mem_size, -mem_size); This corresponds to the case memmap=nn[KMG] and it is not documented in kernel-parameters.txt. Not sure it's needed at all as the same result can be achieved with memmap=nn[KMG]$0. > > > > > > + 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@lists.infradead.org > > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > > > Thanks, > > > > Yunhui > > > > Thanks, > > Yunhui
Hi Mike, On Sun, Jun 23, 2024 at 5:10 PM Mike Rapoport <rppt@kernel.org> wrote: > > On Thu, Jun 20, 2024 at 11:39:41PM -0700, Charlie Jenkins wrote: > > On Fri, Jun 21, 2024 at 02:02:18PM +0800, yunhui cui wrote: > > > Hi Charlie， > > > > > > On Fri, Jun 21, 2024 at 11:10 AM Charlie Jenkins <charlie@rivosinc.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@rivosinc.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 > > I believe you should add RISCV to the list of supported architectures for > these options. Okay, I will update it on v2. > > > > > > > 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? > > > > The mips implementation is very close to being the same, but I am not > > sure if the differences would prevent standardization. xtensa and now > > riscv would have identical implementations though so a generic memmap > > implementation could be only applied to those two archs. > > The memmap= is generally a hack to workaround issues with how firmware > describes memory to the kernel so in a way it belongs to arch/ code. > > mips and xtensa already have different views on how this should be treated, > not mentioning x86 that handles memmap= on e820 level rather than with > memblock. OK, let's continue to put it in arch/. > > > > > > > > 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); > > This will add a region to memblock.reserved, but if there is no memory > there this information won't be available after boot, e.g. there won't be > struct pages with PG_Reserved for this region. > Is this your intention? Yeah, we should use memmap to reserve in the known memory range. > > > > > > > > + break; > > > > > > > + > > > > > > > + case 0: > > > > > > > + memblock_reserve(mem_size, -mem_size); > > This corresponds to the case memmap=nn[KMG] and it is not documented in > kernel-parameters.txt. > > Not sure it's needed at all as the same result can be achieved with > memmap=nn[KMG]$0. OK, just use case '$', because memblock_reserve() converts size to unsigned. > > > > > > > > + 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@lists.infradead.org > > > > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > > > > > Thanks, > > > > > Yunhui > > > > > > Thanks, > > > Yunhui > > -- > Sincerely yours, > Mike. Thanks, Yunhui
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);
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 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@bytedance.com> --- arch/riscv/mm/init.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)