From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Chen, Tiejun" <tiejun.chen@intel.com>
Subject: Re: [RFC][PATCH 1/5] xen:x86: record RMRR mappings
Date: Mon, 11 Aug 2014 11:04:21 +0800
Message-ID: <53E832B5.9040607@intel.com>
References: <1407409371-31728-1-git-send-email-tiejun.chen@intel.com>
	<1407409371-31728-2-git-send-email-tiejun.chen@intel.com>
	<53E50AA1020000780002AB14@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <53E50AA1020000780002AB14@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: kevin.tian@intel.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, yang.z.zhang@intel.com
List-Id: xen-devel@lists.xenproject.org

On 2014/8/8 23:36, Jan Beulich wrote:
>>>> On 07.08.14 at 13:02, <tiejun.chen@intel.com> wrote:
>> +/* Record RMRR mapping to ready expose VM. */
>> +static int __init rmrr_e820_register(struct acpi_rmrr_unit *rmrr)
>> +{
>> +    static int i = 0;
>> +
>> +    rmrr_e820.map[i].addr = rmrr->base_address;
>> +    rmrr_e820.map[i].size = rmrr->end_address - rmrr->base_address;
>> +    rmrr_e820.map[i].type = E820_RESERVED;
>> +    rmrr_e820.nr_map = i;
>> +    i++;
>> +    return 0;
>> +}
>
> As already said elsewhere, the piggybacking on the E820 structure
> isn't suitable here due to that ones limited size.

Are you saying the limited number of e820entry? If yes, I don't think 
this would be limited here.

Because struct e820map always define this as follows,

#define E820MAX 128

struct e820map {
     unsigned int nr_map;
     struct e820entry map[E820MAX];
};

>
> Also, just as general remarks,
> - "i" should be unsigned and placed in __initdata
> - "i" anyway is redundant with rmrr_e820.nr_map
> - there should be a blank line between the last "ordinary"
>    and the return statements
>

I will try to refine these stuffs next revision.

Thanks
Tiejun

> Jan
>
>