[Xen-devel] Re: [Xen-changelog] [xen-unstable] kunmap_atomic() must zap the PTE to avoid dangling references

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Fri, Jun 30, 2006 at 03:40:22PM +0000, Xen patchbot-unstable wrote:

> # HG changeset patch
> # User kfraser@xxxxxxxxxxxxxxxxxxxxxxx
> # Node ID 4260eb8c08740de0000081c61a6237ffcb95b2d5
> # Parent  5fa6c1723e08d6395fecc427f372d008006b2b4c
> kunmap_atomic() must zap the PTE to avoid dangling references
> when attempting to free memory back to Xen. We can implement
> something more efficient in future.
> 
> Also add debug print message if guest tries to free 'in use'
> memory. We'll make it a real guest-visible error in future.
...
> diff -r 5fa6c1723e08 -r 4260eb8c0874 xen/common/memory.c
> --- a/xen/common/memory.c     Wed Jun 28 17:56:34 2006 +0100
> +++ b/xen/common/memory.c     Wed Jun 28 18:17:41 2006 +0100
> @@ -169,6 +169,15 @@ guest_remove_page(
>              
>      if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
>          put_page(page);
> +
> +    if ( unlikely((page->count_info & PGC_count_mask) != 1) )
> +    {
> +        /* We'll make this a guest-visible error in future, so take heed! */
> +        DPRINTK("Dom%d freeing in-use page %lx (pseudophys %lx):"
> +                " count=%x type=%lx\n",
> +                d->domain_id, mfn, get_gpfn_from_mfn(mfn),
> +                page->count_info, page->u.inuse.type_info);
> +    }
>  
>      guest_physmap_remove_page(d, gmfn, mfn);

The above check results in false positive on Xen/IA64
because a page 'in use' has reference count 2 on Xen/IA64.
- a page is assigned to guest domain's psudo physical address space.
  This is decremented by guest_physmap_remove_page()
- a page is allocated for a domain.
  This is decremented by the following put_page()

Work around is needed.
I think the option B. is better, but I'm not sure.
Which option is better?

A. just #ifdef.
   poor solution.
B. define an arch-specific constant like PGC_removable
C. define an arch-specific checking macro or function like
   is_page_removable().
D. move the check under guest_physical_remove_page().
   Probably this option isn't good because we may want to detect an error
   before doing complex thing by guest_physimap_remove_page().
E. others.

-- 
yamahata

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