From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752251AbdKMIKq (ORCPT ); Mon, 13 Nov 2017 03:10:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54312 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751653AbdKMIKp (ORCPT ); Mon, 13 Nov 2017 03:10:45 -0500 Date: Mon, 13 Nov 2017 16:10:41 +0800 From: Baoquan He To: Chao Fan Cc: linux-kernel@vger.kernel.org, x86@kernel.org, hpa@zytor.com, tglx@linutronix.de, mingo@redhat.com, keescook@chromium.org, yasu.isimatu@gmail.com, indou.takao@jp.fujitsu.com, caoj.fnst@cn.fujitsu.com, douly.fnst@cn.fujitsu.com Subject: Re: [PATCH v2 1/4] kaslr: parse the extended movable_node=nn[KMG]@ss[KMG] Message-ID: <20171113081041.GD10474@x1> References: <20171101113203.27741-1-fanc.fnst@cn.fujitsu.com> <20171101113203.27741-2-fanc.fnst@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171101113203.27741-2-fanc.fnst@cn.fujitsu.com> User-Agent: Mutt/1.7.0 (2016-08-17) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 13 Nov 2017 08:10:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Chao, Please think more on your patches, better to discuss with your colleagues and ask them to help review before your post. On 11/01/17 at 07:32pm, Chao Fan wrote: > Extend the movable_node to movable_node=nn[KMG]@ss[KMG]. Firstly, apparently we can't make use of movable_node kernel parameter and extend it to pass in the immovable_nodes. In fact we are passing in the memory ranges which can be used for kernel data, not sure if kernelcore= is OK, or we can make a new one. Think more about this, or consult your colleagues, FJ has many experts on mem hotplugging. Choose an proper one or create a new one. > > Since in current code, kaslr may choose the memory region in movable > nodes to extract kernel, which will make the nodes can't be hot-removed. > To solve it, we can specify the region in immovable node. Create > immovable_mem to store the regions in immovable_mem, where should be > chosen by kaslr. > > Multiple regions can be specified, comma delimited. > Considering the usage of memory, only support for 4 regions. > 4 regions contains 2 nodes at least, enough for kernel to extract. > > Signed-off-by: Chao Fan > --- > arch/x86/boot/compressed/kaslr.c | 76 +++++++++++++++++++++++++++++++++++++++- > 1 file changed, 75 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c > index 17818ba6906f..0a591c0023f1 100644 > --- a/arch/x86/boot/compressed/kaslr.c > +++ b/arch/x86/boot/compressed/kaslr.c > @@ -107,6 +107,15 @@ enum mem_avoid_index { > > static struct mem_vector mem_avoid[MEM_AVOID_MAX]; > > +/* Only supporting at most 4 immovable memory regions with kaslr */ > +#define MAX_IMMOVABLE_MEM 4 > + > +/* Store the memory regions in immovable node */ > +static struct mem_vector immovable_mem[MAX_IMMOVABLE_MEM]; > + > +/* The immovable regions user specify, not more than 4 */ > +static int num_immovable_region; I am fine we support 4 immvoable_mem for now. Please discuss with yor colleagues, make sure if 4 is OK. > + > static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two) > { > /* Item one is entirely before item two. */ > @@ -167,6 +176,38 @@ parse_memmap(char *p, unsigned long long *start, unsigned long long *size) > return -EINVAL; > } > > +static int parse_immovable_mem(char *p, > + unsigned long long *start, > + unsigned long long *size) > +{ > + char *oldp; > + > + if (!p) > + return -EINVAL; > + > + oldp = p; > + *size = memparse(p, &p); > + if (p == oldp) > + return -EINVAL; > + > + /* We support nn[KMG]@ss[KMG] and nn[KMG]. */ > + switch (*p) { > + case '@': > + *start = memparse(p + 1, &p); > + return 0; > + default: > + /* > + * If w/o offset, only size specified, movable_node=nn[KMG] > + * has the same behaviour as movable_node=nn[KMG]@0. It means > + * the region starts from 0. > + */ > + *start = 0; > + return 0; > + } > + > + return -EINVAL; > +} > + > static void mem_avoid_memmap(char *str) > { > static int i; > @@ -206,6 +247,36 @@ static void mem_avoid_memmap(char *str) > memmap_too_large = true; > } > > +#ifdef CONFIG_MEMORY_HOTPLUG > +static void mem_mark_immovable(char *str) I know you try to imitate the function memblock_mark_hotplug(), but do you really think you are marking the immovable mem regions? In below code? Even if use parse_immovable_mem_regions(), please do not use mem_mark_immovable. > +{ > + static int i; > + > + while (str && (i < MAX_IMMOVABLE_MEM)) { > + int rc; > + unsigned long long start, size; > + char *k = strchr(str, ','); > + > + if (k) > + *k++ = 0; > + > + rc = parse_immovable_mem(str, &start, &size); > + if (rc < 0) > + break; > + str = k; > + > + immovable_mem[i].start = start; > + immovable_mem[i].size = size; > + i++; > + } > + num_immovable_region = i; > +} > +#else > +static inline void mem_mark_immovable(char *str) > +{ > +} > +#endif > + > static int handle_mem_memmap(void) Please think of a better function name when you add immovable memory regions parsing in. Clearly it's not a right name now. > { > char *args = (char *)get_cmd_line_ptr(); > @@ -214,7 +285,8 @@ static int handle_mem_memmap(void) > char *param, *val; > u64 mem_size; > > - if (!strstr(args, "memmap=") && !strstr(args, "mem=")) > + if (!strstr(args, "memmap=") && !strstr(args, "mem=") && > + !strstr(args, "movable_node=")) > return 0; > > tmp_cmdline = malloc(len + 1); > @@ -239,6 +311,8 @@ static int handle_mem_memmap(void) > > if (!strcmp(param, "memmap")) { > mem_avoid_memmap(val); > + } else if (!strcmp(param, "movable_node")) { > + mem_mark_immovable(val); > } else if (!strcmp(param, "mem")) { > char *p = val; > > -- > 2.13.6 > > >