[PATCH] xen/mm: Unmap page for direct mapped domain on decrease reservation

[PATCH] xen/mm: Unmap page for direct mapped domain on decrease reservation

[Julien Grall]

Direct mapped domain needs to retrieve the exact same underlying
physical page when the region is re-populated.

Currently, when the memory reservation for this domain is decreased, the
request is just ignored and the page stayed mapped in the P2M. However,
this make more difficult to spot issue when the domain has not yet mapped
foreign page but trying to access the region.

What we really care for direct mapped domain is to not give back the
page to the allocator. So we can re-enable to direct mapped when the guest
memory region is re-populated.

The rest of the process to remove a page can be safely done. This
also ensures us to stay close to the normal domain memory handling.

At the same time, drop the trailing whitespaces around the code
modified.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Tim Deegan <tim@xen.org>

This helped me to debug 64KB pages when only a part of the page is
actually mapped to foreign memory.

With this patch, direct domain will have the region ballooned unmap
until it decides to map foreign page.
---
 xen/common/memory.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 206955f..fd1f268 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -257,8 +257,16 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
 
     if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
         put_page_and_type(page);
-            
-    if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
+
+    /*
+     * With the lack for iommu on some ARM platform, domain with DMA-capable
+     * device must retrieve the same pfn when the hypercall populate_physmap
+     * is called.
+     *
+     * For this purpose, the page is kept allocated.
+     */
+    if ( !is_domain_direct_mapped(d) &&
+         test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
     guest_physmap_remove_page(d, gmfn, mfn, 0);
@@ -309,13 +317,6 @@ static void decrease_reservation(struct memop_args *a)
              && p2m_pod_decrease_reservation(a->domain, gmfn, a->extent_order) )
             continue;
 
-        /* With the lack for iommu on some ARM platform, domain with DMA-capable
-         * device must retrieve the same pfn when the hypercall
-         * populate_physmap is called.
-         */
-        if ( is_domain_direct_mapped(a->domain) )
-            continue;
-
         for ( j = 0; j < (1 << a->extent_order); j++ )
             if ( !guest_remove_page(a->domain, gmfn + j) )
                 goto out;
-- 
2.1.4

Re: [PATCH] xen/mm: Unmap page for direct mapped domain on decrease reservation

[Jan Beulich]

>>> On 27.10.15 at 12:33, <julien.grall@citrix.com> wrote:
> Direct mapped domain needs to retrieve the exact same underlying
> physical page when the region is re-populated.
> 
> Currently, when the memory reservation for this domain is decreased, the
> request is just ignored and the page stayed mapped in the P2M. However,
> this make more difficult to spot issue when the domain has not yet mapped
> foreign page but trying to access the region.
> 
> What we really care for direct mapped domain is to not give back the
> page to the allocator. So we can re-enable to direct mapped when the guest
> memory region is re-populated.
> 
> The rest of the process to remove a page can be safely done. This
> also ensures us to stay close to the normal domain memory handling.
> 
> At the same time, drop the trailing whitespaces around the code
> modified.
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>

Looks good, just one minor comment (which could be addressed upon
commit):

> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -257,8 +257,16 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
>  
>      if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
>          put_page_and_type(page);
> -            
> -    if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
> +
> +    /*
> +     * With the lack for iommu on some ARM platform, domain with DMA-capable

I think it's "lack of", and I also wonder whether calling out ARM here
is really meaningful.

> +     * device must retrieve the same pfn when the hypercall populate_physmap
> +     * is called.
> +     *
> +     * For this purpose, the page is kept allocated.

..., to match populate_physmap() behavior. (or some such)

Jan

Re: [PATCH] xen/mm: Unmap page for direct mapped domain on decrease reservation

[Julien Grall]

On 27/10/15 13:08, Jan Beulich wrote:
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -257,8 +257,16 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
>>  
>>      if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
>>          put_page_and_type(page);
>> -            
>> -    if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
>> +
>> +    /*
>> +     * With the lack for iommu on some ARM platform, domain with DMA-capable
> 
> I think it's "lack of", and I also wonder whether calling out ARM here
> is really meaningful.
> 
>> +     * device must retrieve the same pfn when the hypercall populate_physmap
>> +     * is called.
>> +     *
>> +     * For this purpose, the page is kept allocated.
> 
> ..., to match populate_physmap() behavior. (or some such)

I saw that your already committed the patch. FWIW, I'm fine with both
changes.

Regards,

-- 
Julien Grall