On Fri, 2011-03-04 at 20:53 +0000, Daniel De Graaf wrote:
> >> Is there some reason the gntdev driver can't just balloon down
> >> (XENMEM_decrease_reservation) to make itself a space to map and then
> >> balloon back up (XENMEM_increase_reservation) after unmap when running
> >> HVM?
> >
> > I recall having problems with doing this last time, but I think other
> > changes
> > to make the balloon driver work in HVM may have fixed the issue. I'll try it
> > again; I think this would be the best solution.
>
> Using the balloon hypercalls does work in my test. Below is a patch that
> uses them directly in gntdev, which is simpler but doesn't handle errors
> in re-ballooning and doesn't take advantage of already-ballooned pages
> from the balloon device. Taking advantage of the balloon device is also
> possible and would be more efficient, but adds another dependency to
> gntdev. Any opinions on which method is preferred? If not, I'll try
> making a patch to implement the balloon-assisted version.
The balloon driver in the 2.6.32 pvops (and all old-style Xen kernels
too) has interfaces which are used by the backend drivers for exactly
this purpose.
It was always a bit of a wart that the backends depended on the balloon
driver in this way. IMHO the core ballooning functionality and kernel
interfaces should be moved into the core kernel under CONFIG_XEN. All
that should remain under CONFIG_XEN_BALLOON is the sysfs and xenbus
integration e.g. the user/admin visible interfaces.
This is somewhat analogous to drivers/xen/{events,grant-table}.c being
core kernel and drivers/xen/{gnt,evt}dev.c being configurable optional
modules.
It's possible that what remains under CONFIG_XEN_BALLOON is such a tiny
amount of code that it's not worth keeping it as a separate option,
although the "depends SYSFS" aspect may make it more worthwhile.
Ian.
>
> 8<-------------------------------------------------------------------------
>
> Grant mappings cause the PFN<->MFN mapping to be lost on the pages
> used for the mapping. Instead of leaking memory, explicitly free it
> to the hypervisor and request the memory back after unmapping. This
> removes the need for the bad-page leak workaround.
>
> Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> ---
> drivers/xen/gntdev.c | 58 +++++++++++++++++++++++++++++++++----------------
> 1 files changed, 39 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index d43ff30..104098a 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -41,6 +41,7 @@
> #include <asm/xen/hypervisor.h>
> #include <asm/xen/hypercall.h>
> #include <asm/xen/page.h>
> +#include <xen/interface/memory.h>
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Derek G. Murray <Derek.Murray@xxxxxxxxxxxx>, "
> @@ -203,27 +204,9 @@ static void gntdev_put_map(struct grant_map *map)
> unmap_grant_pages(map, 0, map->count);
>
> for (i = 0; i < map->count; i++) {
> - uint32_t check, *tmp;
> if (!map->pages[i])
> continue;
> - /* XXX When unmapping in an HVM domain, Xen will
> - * sometimes end up mapping the GFN to an invalid MFN.
> - * In this case, writes will be discarded and reads will
> - * return all 0xFF bytes. Leak these unusable GFNs
> - * until Xen supports fixing their p2m mapping.
> - *
> - * Confirmed present in Xen 4.1-RC3 with HVM source
> - */
> - tmp = kmap(map->pages[i]);
> - *tmp = 0xdeaddead;
> - mb();
> - check = *tmp;
> - kunmap(map->pages[i]);
> - if (check == 0xdeaddead)
> - __free_page(map->pages[i]);
> - else
> - pr_debug("Discard page %d=%ld\n", i,
> - page_to_pfn(map->pages[i]));
> + __free_page(map->pages[i]);
> }
> }
> kfree(map->pages);
> @@ -258,11 +241,19 @@ static int map_grant_pages(struct grant_map *map)
> {
> int i, err = 0;
> phys_addr_t addr;
> + unsigned long frame;
>
> if (!use_ptemod) {
> + struct xen_memory_reservation reservation = {
> + .address_bits = 0,
> + .extent_order = 0,
> + .nr_extents = 1,
> + .domid = DOMID_SELF
> + };
> /* Note: it could already be mapped */
> if (map->map_ops[0].handle != -1)
> return 0;
> + set_xen_guest_handle(reservation.extent_start, &frame);
> for (i = 0; i < map->count; i++) {
> addr = (phys_addr_t)
> pfn_to_kaddr(page_to_pfn(map->pages[i]));
> @@ -271,6 +262,12 @@ static int map_grant_pages(struct grant_map *map)
> map->grants[i].domid);
> gnttab_set_unmap_op(&map->unmap_ops[i], addr,
> map->flags, -1 /* handle */);
> + /* TODO batch hypercall, needs buffer */
> + frame = page_to_pfn(map->pages[i]);
> + err = HYPERVISOR_memory_op(XENMEM_decrease_reservation,
> + &reservation);
> + if (WARN_ON(err <= 0))
> + printk(KERN_INFO "memop returned %d\n", err);
> }
> }
>
> @@ -324,6 +321,29 @@ static int __unmap_grant_pages(struct grant_map *map,
> int offset, int pages)
> map->unmap_ops[offset+i].status);
> map->unmap_ops[offset+i].handle = -1;
> }
> +
> + if (!use_ptemod) {
> + struct xen_memory_reservation reservation = {
> + .address_bits = 0,
> + .extent_order = 0,
> + .nr_extents = 1,
> + .domid = DOMID_SELF
> + };
> + int rc;
> + unsigned long frame;
> + set_xen_guest_handle(reservation.extent_start, &frame);
> + /* TODO batch hypercall, needs buffer */
> + for (i = offset; i < pages + offset; i++) {
> + frame = page_to_pfn(map->pages[i]);
> + rc = HYPERVISOR_memory_op(XENMEM_populate_physmap,
> + &reservation);
> + if (WARN_ON(rc <= 0)) {
> + map->pages[i] = NULL;
> + continue;
> + }
> + }
> + }
> +
> return err;
> }
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home