From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][PATCH 13/13] hvmloader/e820: construct guest e820 table Date: Fri, 15 May 2015 16:47:48 +0800 Message-ID: <5555B2B4.2060609@intel.com> References: <1428657724-3498-1-git-send-email-tiejun.chen@intel.com> <1428657724-3498-14-git-send-email-tiejun.chen@intel.com> <553529850200007800073DB7@mail.emea.novell.com> <55558E20.6020305@intel.com> <5555AD8B020000780007A5AE@mail.emea.novell.com> <555594AA.4040905@intel.com> <5555B4D7020000780007A620@mail.emea.novell.com> <55559C07.6000601@intel.com> <5555BDAF020000780007A679@mail.emea.novell.com> <5555A7A1.9020006@intel.com> <5555C671020000780007A6DD@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5555C671020000780007A6DD@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: tim@xen.org, kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org On 2015/5/15 16:12, Jan Beulich wrote: >>>> On 15.05.15 at 10:00, wrote: >> On 2015/5/15 15:34, Jan Beulich wrote: >>>>>> On 15.05.15 at 09:11, wrote: >>>> On 2015/5/15 14:56, Jan Beulich wrote: >>>>>>>> On 15.05.15 at 08:39, wrote: >>>>>> On 2015/5/15 14:25, Jan Beulich wrote: >>>>>>>>>> On 15.05.15 at 08:11, wrote: >>>>>>>> Even we may separate the >>>>>>>> low memory to construct memory_map.map[]... >>>>>>> >>>>>>> ??? >>>>>> >>>>>> Sorry I just mean that the low memory is not represented with only one >>>>>> memory_map.map[] in some cases. >>>>> >>>>> That's correct. >>>>> >>>> >>>> So just lets keep that original BUG_ON()? >>> >>> In your previous reply you seemed to agree that the BUG_ON() is >>> becoming meaningless. Why do you now suggest to keep it then? >>> >> >> Sorry just let me clear this. >> >> We still need to check this, >> >> (hvm_info->low_mem_pgend << PAGE_SHIFT) < (2u << 20) >> >> Right? I agree the original is really less relevant as you said. > > And I didn't ask you to drop it. All I asked it to amend it with another > BUG_ON() checking what the one above won't cover anymore. > Another point hits me in this case while we're discussing MMIO in another email. We may populate RAM to get enough MMIO in pci_setup() so this means hvm_info->low_mem_pgend would be changed. Furthermore, low_mem_pgend isn't going to keep recording our original lowmem while building domain. So I think this original BUG_ON() is still good to cover this case. But obviously, we need to adjust its associated memory_map.map[x] right now. So what about this? @@ -74,6 +74,7 @@ int build_e820_table(struct e820entry *e820, unsigned int bios_image_base) { unsigned int nr = 0, i, j; + uint64_t low_mem_pgend = hvm_info->low_mem_pgend << PAGE_SHIFT; if ( !lowmem_reserved_base ) lowmem_reserved_base = 0xA0000; @@ -117,9 +118,6 @@ int build_e820_table(struct e820entry *e820, e820[nr].type = E820_RESERVED; nr++; - /* Low RAM goes here. Reserve space for special pages. */ - BUG_ON((hvm_info->low_mem_pgend << PAGE_SHIFT) < (2u << 20)); - /* * Explicitly reserve space for special pages. * This space starts at RESERVED_MEMBASE an extends to cover various @@ -162,6 +160,17 @@ int build_e820_table(struct e820entry *e820, nr++; } + /* Low RAM goes here. Reserve space for special pages. */ + BUG_ON(low_mem_pgend < (2u << 20)); + /* We may need to adjust lowmem end. */ + for ( i = 0; i < memory_map.nr_map; i++ ) + { + uint64_t end = e820[i].addr + e820[i].size; + if ( e820[i].type == E820_RAM && + low_mem_pgend > e820[i].addr && low_mem_pgend < end ) + e820[i].size = low_mem_pgend - e820[i].addr; + } + /* May need to reorder all e820 entries. */ for ( j = 0; j < nr-1; j++ ) { Thanks Tiejun