On Wed, 2007-02-21 at 18:17 -0500, 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)
Could you rename "new_max" to "new_max_pages" so we can keep the units
straight? (I know they use "new_max" in the XEN_DOMCTL_max_mem handler.)
> +{
> + 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);
> + return -EINVAL;
> + }
We won't be called in this case, so this test can be removed.
> + /* 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;
> + }
The 16MB thing is because the 970's large page size is 16MB, and the
kernel uses large pages to map its text. That said, I don't see why this
should be enforced by Xen when setting max_mem (if ever). Stylistically,
I also object to the literals used here.
> + /* 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;
I think the Xen heap is on the small side. Do you have an idea of how
much memory we have available? I suppose we can change it later if we
exhaust the heap.
We had talked about using ints for the p2m array, since that would only
limit us to 44 bits of physical memory. Did you decide to use longs
instead?
> + /* 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;
> + }
This memcpy could be pretty slow; might be better if we could make this
a continuation some day. If you agree, could you add a comment to that
effect?
> + /* 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)));
Here you're initializing the array of longs with an int. Since
INVALID_MFN happens to be uniform (0xffffffff), it will work out, but I
don't think it's ideal coding practice.
> + /* 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));
> +
> + 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)
> +
I hate this undef and define. Here's what I want:
asm-x86/shadow.h
#define guest_physmap_max_mem(d, n) (0)
asm-ia64/shadow.h
#define guest_physmap_max_mem(d, n) (0)
asm-powerpc/shadow.h
extern int guest_physmap_max_mem(struct domain *d, unsigned long
new_max);
--
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
|
Home