On Sat, 4 Jun 2011, James Harper wrote:
> >
> > On 04/06/2011 08:38, "James Harper" <james.harper@xxxxxxxxxxxxxxxx>
> wrote:
> >
> > > Looking past the test_bit call, the next statement does another test
> and
> > > sets last_address_index to 0 and returns NULL. Is this just to
> ensure
> > > that the next access isn't just trivially accepted?
> >
> > Yes, first test is on a potentially stale bucket. Second test is on a
> fresh
> > bucket.
> >
>
> How about the following patch? Is munmap the correct way to unmap or is
> an IOCTL required too?
>
munmap is OK
> The exit condition is what would happen anyway after the remap is done
> and the page is still invalid.
>
> diff --git a/hw/xen_machine_fv.c b/hw/xen_machine_fv.c
> index d02e23f..1ff80bb 100644
> --- a/hw/xen_machine_fv.c
> +++ b/hw/xen_machine_fv.c
> @@ -151,6 +151,24 @@ uint8_t *qemu_map_cache(target_phys_addr_t
> phys_addr, uint8_t lock)
> pentry->next = entry;
> qemu_remap_bucket(entry, address_index);
> } else if (!entry->lock) {
> + if (entry->vaddr_base && entry->paddr_index == address_index &&
> !test_bit(address_offset>>XC_PAGE_SHIFT, entry->valid_mapping))
> + {
> + /* The page was invalid previously. Test if it is valid now
> and only remap if so */
> + xen_pfn_t pfn;
> + int err;
> + void *tmp_vaddr;
> +
> + pfn = phys_addr >> XC_PAGE_SHIFT;
> + tmp_vaddr = xc_map_foreign_bulk(xc_handle, domid,
> PROT_READ|PROT_WRITE, &pfn, &err, 1);
> + if (tmp_vaddr)
> + munmap(tmp_vaddr, PAGE_SIZE);
> +
> + if (!tmp_vaddr || err)
> + {
> + last_address_index = ~0UL;
> + return NULL;
> + }
> + }
> if (!entry->vaddr_base || entry->paddr_index != address_index
> || !test_bit(address_offset>>XC_PAGE_SHIFT, entry->valid_mapping))
> qemu_remap_bucket(entry, address_index);
> }
This is just a matter of aesthetic, but probably something like the
following would be clearer?
if (entry->vaddr_base && entry->paddr_index == address_index) {
if (!test_bit(address_offset>>XC_PAGE_SHIFT, entry->valid_mapping)) {
/* your code + remap bucket */
}
} else {
qemu_remap_bucket(entry, address_index);
}
Could you also please send a similar patch for upstream qemu?
The code should be quite similar in this area, look at xen-mapcache.c.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home