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-devel

Re: [Xen-devel] [PATCH 2/2] xen: Introduce VGA sync dirty bitmap support

To: "anthony.perard@xxxxxxxxxx" <anthony.perard@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 2/2] xen: Introduce VGA sync dirty bitmap support
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Wed, 12 Jan 2011 11:15:20 +0000
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, QEMU-devel <qemu-devel@xxxxxxxxxx>
Delivery-date: Wed, 12 Jan 2011 03:15:54 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1294760223-26151-3-git-send-email-anthony.perard@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1294760223-26151-1-git-send-email-anthony.perard@xxxxxxxxxx> <1294760223-26151-3-git-send-email-anthony.perard@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
On Tue, 11 Jan 2011, anthony.perard@xxxxxxxxxx wrote:
> From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> This patch introduces phys memory client for Xen.
> 
> Only sync dirty_bitmap and set_memory are actually implemented.
> migration_log will stay empty for the moment.
> 
> Xen can only log one range for bit change, so only the range in the
> first call will be synced.

The patch looks much better than I expected, one of the benefits of
reusing the ramblock architecture!

 
> +static XenPhysmap *link_exist(target_phys_addr_t start_addr)
> +{
> +    XenPhysmap *physmap = NULL;
> +
> +    start_addr = TARGET_PAGE_ALIGN(start_addr);
> +    QLIST_FOREACH(physmap, &xen_physmap, list) {
> +        if (physmap->size > 0 && physmap->start_addr == start_addr) {
> +            return physmap;
> +        }
> +    }
> +    return NULL;
> +}
> +
> +#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 340
> +static int already_physmapped(target_phys_addr_t phys_offset)
> +{
> +    XenPhysmap *physmap = NULL;
> +
> +    phys_offset = TARGET_PAGE_ALIGN(phys_offset);
> +    QLIST_FOREACH(physmap, &xen_physmap, list) {
> +        if (physmap->size > 0 && physmap->phys_offset <= phys_offset &&
> +            phys_offset <= (physmap->phys_offset + physmap->size)) {
> +            return 1;
> +        }
> +    }
> +    return 0;
> +}
> +

why are you searching for an address within a range in
already_physmapped while you are just looking for a simple match in
link_exist? Shouldn't it be the same in both?


> +static int xen_sync_dirty_bitmap(target_phys_addr_t start_addr,
> +                                 ram_addr_t size)
> +{
> +    target_phys_addr_t npages = size >> TARGET_PAGE_BITS;
> +    target_phys_addr_t vram_offset = 0;
> +    const int width = sizeof(unsigned long) * 8;
> +    unsigned long bitmap[(npages + width - 1) / width];
> +    int rc, i, j;
> +    XenPhysmap *physmap = NULL;
> +
> +    physmap = link_exist(start_addr);
> +    if (physmap) {
> +        vram_offset = physmap->phys_offset;
> +    } else {
> +        vram_offset = start_addr;
> +    }

link_exist is always supposed to return a value here anyway, right?
With the current code is not possible to get here without a mapping, I
think.
However if you are trying to make xen_sync_dirty_bitmap more generic
you also need to handle the case in which start_addr is contained within
an already mapped range.


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