From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1759243AbdAEBAl (ORCPT <rfc822;w@1wt.eu>);
        Wed, 4 Jan 2017 20:00:41 -0500
Received: from mx1.redhat.com ([209.132.183.28]:60968 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751115AbdAEBAi (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 4 Jan 2017 20:00:38 -0500
Date: Thu, 5 Jan 2017 09:00:35 +0800
From: Baoquan He <bhe@redhat.com>
To: Dave Jiang <dave.jiang@intel.com>
Cc: tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
        linux-nvdimm@ml01.01.org, x86@kernel.org, david@fromorbit.com,
        linux-kernel@vger.kernel.org, dan.j.williams@intel.com
Subject: Re: [PATCH v4] x86: fix kaslr and memmap collision
Message-ID: <20170105010035.GC6937@x1>
References: <148347651968.64111.7765172823804130174.stgit@djiang5-desk3.ch.intel.com>
 <20170104023728.GC15788@x1>
 <f91679bd-f728-d3aa-e943-45fb42d2e20b@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f91679bd-f728-d3aa-e943-45fb42d2e20b@intel.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.30]); Thu, 05 Jan 2017 01:00:39 +0000 (UTC)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 01/04/17 at 10:06am, Dave Jiang wrote:
> On 01/03/2017 07:37 PM, Baoquan He wrote:
> >>  #include <generated/compile.h>
> >>  #include <linux/module.h>
> >> @@ -61,9 +62,16 @@ enum mem_avoid_index {
> >>  	MEM_AVOID_INITRD,
> >>  	MEM_AVOID_CMDLINE,
> >>  	MEM_AVOID_BOOTPARAMS,
> >> +	MEM_AVOID_MEMMAP1,
> >> +	MEM_AVOID_MEMMAP2,
> >> +	MEM_AVOID_MEMMAP3,
> >> +	MEM_AVOID_MEMMAP4,
> > 
> > This looks not good. Could it be done like fixed_addresses?
> > Something like:
> > 
> > 	MEM_AVOID_MEMMAP_BEGIN,
> > 	MEM_AVOID_MEMMAP_BEGIN + MEM_AVOID_MAX -1,
> > 
> > Please point it out to me if there's some existing code in kernel like
> > your way, I can also accept it.
> 
> I think you mean:
> MEM_AVOID_MEMMAP_BEGIN + MAX_MEMMAP_REGIONS - 1

Ah, yes. Sorry for this.

> 
> I will change
> 
> 
> > 
> >>  	MEM_AVOID_MAX,
> >>  };
> >>  
> >> +/* only supporting at most 4 memmap regions with kaslr */
> > And here, "Only supporting at most 4 un-usable memmap regions with kaslr"?
> > Surely this is based on if you will ignore the usable memory and do not
> > store it as 0. And also the log need be changed too accordingly.
> >> +#define MAX_MEMMAP_REGIONS	4
...
> >> +static int
> >> +parse_memmap(char *p, unsigned long long *start, unsigned long long *size)
> >> +{
> >> +	char *oldp;
> >> +
> >> +	if (!p)
> >> +		return -EINVAL;
> >> +
> >> +	/* we don't care about this option here */
> >> +	if (!strncmp(p, "exactmap", 8))
> >> +		return -EINVAL;
> >> +
> >> +	oldp = p;
> >> +	*size = _memparse(p, &p);
> >> +	if (p == oldp)
> >> +		return -EINVAL;
> >> +
> >> +	switch (*p) {
> >> +	case '@':
> >> +		/* skip this region, usable */
> >> +		*start = 0;
> >> +		*size = 0;
> >> +		return 0;
> > How about direclty return if nn@ss? Seems no need to waste one mem avoid
> > region slot. In fact even amount of usable memory regions are provided to
> > 100, it won't impact that you want to specify a reserve memmap region if
> > you skip it direclty. Personal opinion.
> 
> We are not wasting the slot. If you look mem_avoid_memmap() where I call
> the function, it will skip with a continue if size == 0 without
> incrementing the 'i' counter. That will skip all the nn@ss regions
> without counting against the max avoid mapping.

Yes, indeed. Sorry, I didn't read the code carefully.

Thanks
Baoquan