Re: [Xen-devel] [RFC][PATCH 13/13] hvmloader/e820: construct guest e820 table

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2015/5/15 16:12, Jan Beulich wrote:
> > > > On 15.05.15 at 10:00, <tiejun.chen@xxxxxxxxx> wrote:
> > On 2015/5/15 15:34, Jan Beulich wrote:
> > > > > > On 15.05.15 at 09:11, <tiejun.chen@xxxxxxxxx> wrote:
> > > > On 2015/5/15 14:56, Jan Beulich wrote:
> > > > > > > > On 15.05.15 at 08:39, <tiejun.chen@xxxxxxxxx> wrote:
> > > > > > On 2015/5/15 14:25, Jan Beulich wrote:
> > > > > > > > > > On 15.05.15 at 08:11, <tiejun.chen@xxxxxxxxx> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel