From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Chen, Tiejun" <tiejun.chen@intel.com>
Subject: Re: [v9][PATCH 07/16] hvmloader/e820: construct guest
	e820 table
Date: Fri, 17 Jul 2015 23:22:21 +0800
Message-ID: <55A91DAD.7030900@intel.com>
References: <1437093920-11472-1-git-send-email-tiejun.chen@intel.com>
	<1437093920-11472-8-git-send-email-tiejun.chen@intel.com>
	<55A8CD740200007800092545@mail.emea.novell.com>
	<55A8C65C.1020804@intel.com>
	<55A8FA26020000780009269C@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: <55A8FA26020000780009269C@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: Wei Liu <wei.liu2@citrix.com>, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, xen-devel@lists.xen.org, Keir Fraser <keir@xen.org>
List-Id: xen-devel@lists.xenproject.org

On 2015/7/17 18:50, Jan Beulich wrote:
>>>> On 17.07.15 at 11:09, <tiejun.chen@intel.com> wrote:
>>> And then of course there's the question of whether "nr" is really
>>> the right upper loop bound here: Just prior to this you added
>>> the hypervisor supplied entries - why would you need to iterate
>>> over them here? I.e. I'd see this better be moved ahead of that
>>> other code.
>>>
>>
>> Sounds you mean I should sync low/high memory in memory_map.map[]
>> beforehand and then fulfill e820 like this,
>
> Why would you want/need to sync into memory_map.map[]?

But actually I just felt this make our process clear.

> That's certainly not what I suggested.
>

Do you mean I should check low/high mem before we add the hypervisor 
supplied entries like this?

+    for ( i = nr-1; i > memory_map.nr_map; i-- )
+    {
+        uint64_t end = e820[i].addr + e820[i].size;
+
+        if ( e820[i].type == E820_RAM &&
+             low_mem_end > e820[i].addr && low_mem_end < end )
+        {
+            add_high_mem = end - low_mem_end;
+            e820[i].size = low_mem_end - e820[i].addr;
+            break;
+        }
+    }
+
+    /* And then we also need to adjust highmem. */
+    if ( add_high_mem )
+    {
+        /* Modify the existing highmem region if it exists. */
+        for ( i = nr-1 ; i > memory_map.nr_map; i-- )
+        {
+            if ( e820[i].type == E820_RAM &&
+                 e820[i].addr == ((uint64_t)1 << 32))
+            {
+                e820[i].size += add_high_mem;
+                break;
+            }
+        }
+
+        /* If there was no highmem region, just create one. */
+        if ( i == memory_map.nr_map )
+        {
+            e820[nr].addr = ((uint64_t)1 << 32);
+            e820[nr].size = add_high_mem;
+            e820[nr].type = E820_RAM;
+            i = nr;
+            nr++;
+        }
+
+        /* A sanity check if high memory is broken. */
+        BUG_ON( high_mem_end != e820[i].addr + e820[i].size);
+    }

Thanks
Tiejun