[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 1/9] libxc: reorganize domain builder guest memory allocator



On 10/28/2015 04:32 PM, Wei Liu wrote:
On Tue, Oct 13, 2015 at 03:11:10PM +0200, Juergen Gross wrote:
Guest memory allocation in the domain builder of libxc is done via
virtual addresses only. In order to be able to support preallocated
areas not virtually mapped reorganize the memory allocator to keep
track of allocated pages globally and in allocated segments.

This requires an interface change of the allocate callback of the
domain builder which currently is using the last mapped virtual
address as a parameter. This is no problem as the only user of this
callback is stubdom/grub/kexec.c using this virtual address to
calculate the last used pfn.

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
  stubdom/grub/kexec.c         |   6 +--
  tools/libxc/include/xc_dom.h |  11 +++--
  tools/libxc/xc_dom_core.c    | 101 +++++++++++++++++++++++++++++--------------
  3 files changed, 75 insertions(+), 43 deletions(-)

diff --git a/stubdom/grub/kexec.c b/stubdom/grub/kexec.c
index 0b2f4f3..2300318 100644
--- a/stubdom/grub/kexec.c
+++ b/stubdom/grub/kexec.c
@@ -100,9 +100,9 @@ static void do_exchange(struct xc_dom_image *dom, xen_pfn_t 
target_pfn, xen_pfn_
      dom->p2m_host[target_pfn] = source_mfn;
  }

-int kexec_allocate(struct xc_dom_image *dom, xen_vaddr_t up_to)
+int kexec_allocate(struct xc_dom_image *dom)
  {
-    unsigned long new_allocated = (up_to - dom->parms.virt_base) / PAGE_SIZE;
+    unsigned long new_allocated = dom->pfn_alloc_end - dom->rambase_pfn;
      unsigned long i;

      pages = realloc(pages, new_allocated * sizeof(*pages));
@@ -319,8 +319,6 @@ void kexec(void *kernel, long kernel_size, void *module, 
long module_size, char

      /* Make sure the bootstrap page table does not RW-map any of our current
       * page table frames */
-    kexec_allocate(dom, dom->virt_pgtab_end);
-

Does this mean pvgrub is now able to use this shiny new feature?

That's the plan. I have to admit I didn't test this. And don't mix this
up with grub-xen (I'm just working on that beast).


      if ( (rc = xc_dom_update_guest_p2m(dom))) {
          grub_printf("xc_dom_update_guest_p2m returned %d\n", rc);
          errnum = ERR_BOOT_FAILURE;
diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index e52b023..878dc52 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -29,6 +29,7 @@ struct xc_dom_seg {
      xen_vaddr_t vstart;
      xen_vaddr_t vend;
      xen_pfn_t pfn;
+    xen_pfn_t pages;
  };

  struct xc_dom_mem {
@@ -90,6 +91,7 @@ struct xc_dom_image {
      xen_pfn_t xenstore_pfn;
      xen_pfn_t shared_info_pfn;
      xen_pfn_t bootstack_pfn;
+    xen_pfn_t pfn_alloc_end;
      xen_vaddr_t virt_alloc_end;
      xen_vaddr_t bsd_symtab_start;

@@ -177,7 +179,7 @@ struct xc_dom_image {
      /* kernel loader */
      struct xc_dom_arch *arch_hooks;
      /* allocate up to virt_alloc_end */

I think you need to update this comment, too.

Hmm, yes.


-    int (*allocate) (struct xc_dom_image * dom, xen_vaddr_t up_to);
+    int (*allocate) (struct xc_dom_image * dom);

      /* Container type (HVM or PV). */
      enum {
@@ -361,14 +363,11 @@ static inline void *xc_dom_seg_to_ptr_pages(struct 
xc_dom_image *dom,
                                        struct xc_dom_seg *seg,
                                        xen_pfn_t *pages_out)
  {
-    xen_vaddr_t segsize = seg->vend - seg->vstart;
-    unsigned int page_size = XC_DOM_PAGE_SIZE(dom);
-    xen_pfn_t pages = (segsize + page_size - 1) / page_size;
      void *retval;

-    retval = xc_dom_pfn_to_ptr(dom, seg->pfn, pages);
+    retval = xc_dom_pfn_to_ptr(dom, seg->pfn, seg->pages);

-    *pages_out = retval ? pages : 0;
+    *pages_out = retval ? seg->pages : 0;
      return retval;
  }

diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index fbe4464..b1d7890 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -535,56 +535,75 @@ void *xc_dom_pfn_to_ptr_retcount(struct xc_dom_image 
*dom, xen_pfn_t pfn,
      return phys->ptr;
  }

-int xc_dom_alloc_segment(struct xc_dom_image *dom,
-                         struct xc_dom_seg *seg, char *name,
-                         xen_vaddr_t start, xen_vaddr_t size)
+static int xc_dom_chk_alloc_pages(struct xc_dom_image *dom, char *name,
+                                  xen_pfn_t pages)
  {
      unsigned int page_size = XC_DOM_PAGE_SIZE(dom);
-    xen_pfn_t pages = (size + page_size - 1) / page_size;
-    xen_pfn_t pfn;
-    void *ptr;

-    if ( start == 0 )
-        start = dom->virt_alloc_end;
+    if ( pages > dom->total_pages || /* multiple test avoids overflow probs */
+         dom->pfn_alloc_end - dom->rambase_pfn > dom->total_pages ||
+         pages > dom->total_pages - dom->pfn_alloc_end + dom->rambase_pfn )
+    {
+        xc_dom_panic(dom->xch, XC_OUT_OF_MEMORY,
+                     "%s: segment %s too large (0x%"PRIpfn" > "
+                     "0x%"PRIpfn" - 0x%"PRIpfn" pages)", __FUNCTION__, name,
+                     pages, dom->total_pages,
+                     dom->pfn_alloc_end - dom->rambase_pfn);
+        return -1;
+    }
+
+    dom->pfn_alloc_end += pages;
+    dom->virt_alloc_end += pages * page_size;

-    if ( start & (page_size - 1) )
+    return 0;
+}
+
+static int xc_dom_alloc_pad(struct xc_dom_image *dom, xen_vaddr_t end)
+{
+    unsigned int page_size = XC_DOM_PAGE_SIZE(dom);
+    xen_pfn_t pages;
+
+    if ( end & (page_size - 1) )
      {
          xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
                       "%s: segment start isn't page aligned (0x%" PRIx64 ")",

"segment end"?

No. This function is called to add a padding allocation before the start
of a new segment, which has to start at page boundary. Maybe a comment
should clarify this. :-)


-                     __FUNCTION__, start);
+                     __FUNCTION__, end);
          return -1;
      }
-    if ( start < dom->virt_alloc_end )
+    if ( end < dom->virt_alloc_end )
      {
          xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
                       "%s: segment start too low (0x%" PRIx64 " < 0x%" PRIx64

Ditto.

Ditto. ;-)


A major part of this patch looks like refactoring to me. And to the
best of my knowledge it seems to be doing the right thing.

Thanks.


Juergen


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.