WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ppc-devel

Re: [XenPPC] [PATCH 3 of 6] [PATCH] xen: implement guest_physmap_max_mem

To: Ryan Harper <ryanh@xxxxxxxxxx>
Subject: Re: [XenPPC] [PATCH 3 of 6] [PATCH] xen: implement guest_physmap_max_mem for ppc
From: Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx>
Date: Thu, 22 Feb 2007 12:24:56 +0100
Cc: xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 22 Feb 2007 04:23:44 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <35fd77200dff7e73fe39.1172103421@xxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-ppc-devel-request@lists.xensource.com?subject=help>
List-id: Xen PPC development <xen-ppc-devel.lists.xensource.com>
List-post: <mailto:xen-ppc-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ppc-devel>, <mailto:xen-ppc-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ppc-devel>, <mailto:xen-ppc-devel-request@lists.xensource.com?subject=unsubscribe>
References: <35fd77200dff7e73fe39.1172103421@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-ppc-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 1.5.0.9 (X11/20061215)
Ryan Harper wrote:
4 files changed, 72 insertions(+)
xen/arch/powerpc/domain.c        |   60 ++++++++++++++++++++++++++++++++++++++
xen/arch/powerpc/domain_build.c  |    5 +++
xen/include/asm-powerpc/domain.h |    3 +
xen/include/asm-powerpc/shadow.h |    4 ++


# HG changeset patch
# User Ryan Harper <ryanh@xxxxxxxxxx>
# Date 1172103252 21600
# Node ID 35fd77200dff7e73fe3959b5dbfa6088c691c502
# Parent  84ec1b4d5cd50cc9d49202eb978a4715c4780e28
[PATCH] xen: implement guest_physmap_max_mem for ppc

Signed-off-by: Ryan Harper <ryanh@xxxxxxxxxx>

diff -r 84ec1b4d5cd5 -r 35fd77200dff xen/arch/powerpc/domain.c
--- a/xen/arch/powerpc/domain.c Wed Feb 21 18:14:12 2007 -0600
+++ b/xen/arch/powerpc/domain.c Wed Feb 21 18:14:12 2007 -0600
@@ -33,6 +33,7 @@
 #include <asm/htab.h>
 #include <asm/current.h>
 #include <asm/hcalls.h>
+#include <asm/shadow.h>
 #include "rtas.h"
 #include "exceptions.h"

@@ -347,3 +348,62 @@ void idle_loop(void)
         do_softirq();
     }
 }
+
+int do_guest_physmap_max_mem(struct domain *d, unsigned long new_max)
+{
+    ulong *p2m_array = NULL;
+    ulong *p2m_old = NULL;
+    ulong p2m_pages;
+    ulong copy_end = 0;
+
+    /* we don't handle shrinking max_pages */
+    if ( new_max < d->max_pages )
+    {
+        printk("Can't shrink %d max_mem\n", d->domain_id);

Just some wording, but maybe printk("Can't shrink max_mem of domain %d\n", d->domain_id); would prevent some users from mis-interpreting the number as an unit of memory size.

+        return -EINVAL;
+    }
+
+    /* our work is done here */
+    if ( new_max == d->max_pages )
+        return 0;
+
+    /* check new_max pages is 16MiB aligned */
+    if ( new_max & ((1<<12)-1) )
+    {
+        printk("Must be 16MiB aligned increments\n");
+        return -EACCES;
+    }
+
+    /* make a p2m array of new_max mfns */
+    p2m_pages = (new_max * sizeof(ulong)) >> PAGE_SHIFT;
+    /* XXX: could use domheap anonymously */
+    p2m_array = alloc_xenheap_pages(get_order_from_pages(p2m_pages));
+    if ( p2m_array == NULL )
+        return -ENOMEM;
+
+    /* copy old mappings into new array if any */
+    if ( d->arch.p2m != NULL )
+    {
+        /* mark where the copy will stop (which page) */
+        copy_end = d->max_pages;
+
+        /* memcpy takes size in bytes */
+        memcpy(p2m_array, d->arch.p2m, (d->max_pages * sizeof(ulong)));
+
+        /* keep a pointer to the old array */
+        p2m_old = d->arch.p2m;
+    }
+
+    /* mark remaining (or all) mfn as INVALID_MFN, memset takes size in bytes 
*/
+    memset(p2m_array+copy_end, (int)INVALID_MFN,
+           (((ulong)(new_max - d->max_pages)) * sizeof(ulong)));
+
+    /* set p2m pointer */
+    d->arch.p2m = p2m_array;
+
+    /* free old p2m array if present */
+    if ( p2m_old )
+        free_xenheap_pages(d->arch.p2m, get_order_from_pages(d->max_pages));
Maybe I just don't get it right because I'm new in this area, but if an old mapping exists you do here summarized:
1. save old d->arch.p2m in p2m_old
2. create a new p2m_array including the old mapping as copy
3. assigning the new array to d->arch.p2m
4. ?? if an old mapping was present you free the new one in d->arch.p2m ??
=> this would leave one unreferenced heap allocation in memory (p2m_old) without a chance to free it after the local variable p2m_old disappears and the actively used table d->arch.p2m point to freed heap.
I assume the freeing code should be

+    /* free old p2m array if present */
+    if ( p2m_old )
+        free_xenheap_pages(p2m_old, get_order_from_pages(d->max_pages));


One more question, should there not be an update of the max_pages value in the domain structure before returning to ensure next time this (or other) function is called it can operate on the new size of the mapping (e.g. in pfn2mfn you would not get a mfn for all mappings that are located in the area now extended because there is a if(pfn<d->max_pages) and there are a lot of comparisons based on max_pages around there)? As suggestion add an extra line before the return like:
+ d->max_pages = new_max;

And at last a question because I don't know the environment where this function is called from. May this function be called concurrently for the same "struct domain" ? If it would be possible you should add some locking to prevent races.

+
+    return 0;
+}
diff -r 84ec1b4d5cd5 -r 35fd77200dff xen/arch/powerpc/domain_build.c
--- a/xen/arch/powerpc/domain_build.c   Wed Feb 21 18:14:12 2007 -0600
+++ b/xen/arch/powerpc/domain_build.c   Wed Feb 21 18:14:12 2007 -0600
@@ -28,6 +28,7 @@
 #include <xen/shadow.h>
 #include <xen/domain.h>
 #include <xen/version.h>
+#include <xen/shadow.h>
 #include <asm/processor.h>
 #include <asm/papr.h>
 #include <public/arch-powerpc.h>
@@ -159,6 +160,10 @@ int construct_dom0(struct domain *d,
         if (dom0_nrpages < CONFIG_MIN_DOM0_PAGES)
             dom0_nrpages = CONFIG_MIN_DOM0_PAGES;
     }
+
+    /* set DOM0 max mem, triggering p2m table creation */
+    if ( (guest_physmap_max_mem(d, dom0_nrpages)) != 0 )
+        panic("Failed to set DOM0 max mem value\n");

     /* By default DOM0 is allocated all available memory. */
     d->max_pages = dom0_nrpages;
diff -r 84ec1b4d5cd5 -r 35fd77200dff xen/include/asm-powerpc/domain.h
--- a/xen/include/asm-powerpc/domain.h  Wed Feb 21 18:14:12 2007 -0600
+++ b/xen/include/asm-powerpc/domain.h  Wed Feb 21 18:14:12 2007 -0600
@@ -46,6 +46,9 @@ struct arch_domain {

     /* I/O-port access bitmap mask. */
     u8 *iobmp_mask;       /* Address of IO bitmap mask, or NULL.      */
+
+    /* P2M mapping array */
+    ulong *p2m;

     uint large_page_sizes;
     uint large_page_order[4];
diff -r 84ec1b4d5cd5 -r 35fd77200dff xen/include/asm-powerpc/shadow.h
--- a/xen/include/asm-powerpc/shadow.h  Wed Feb 21 18:14:12 2007 -0600
+++ b/xen/include/asm-powerpc/shadow.h  Wed Feb 21 18:14:12 2007 -0600
@@ -37,6 +37,10 @@ extern void guest_physmap_remove_page(
 extern void guest_physmap_remove_page(
     struct domain *d, unsigned long gpfn, unsigned long mfn);

+int do_guest_physmap_max_mem(struct domain *d, unsigned long new_max);
+#undef guest_physmap_max_mem
+#define guest_physmap_max_mem(d, n) do_guest_physmap_max_mem(d, n)
+
 extern void shadow_drop_references(
     struct domain *d, struct page_info *page);


_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel

--

Grüsse / regards, Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt@xxxxxxxxxxxxxxxxxxx
Ehrhardt@xxxxxxxxxx

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen Geschäftsführung: Herbert Kircher Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel

<Prev in Thread] Current Thread [Next in Thread>