Re: [PATCH v4 08/11] xen/arm: Handle remove foreign mapping

[Julien Grall <julien.grall@linaro.org>]

On 12/16/2013 11:51 AM, Tim Deegan wrote:
> At 19:37 +0000 on 13 Dec (1386959858), Julien Grall wrote:
>> @@ -693,12 +694,21 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>               return rc;
>>           }
>>
>> -        page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
>> +        /*
>> +         * If autotranslate guest, (eg pvh), the gfn could be mapped to a mfn
>> +         * from foreign domain by the user space tool during domain creation.
>> +         * We need to check for that, free it up from the p2m, and release
>> +         * refcnt on it. In such a case, page would be NULL and the following
>> +         * call would not have refcnt'd the page.
>> +         */
>> +        page = get_page_from_gfn(d, xrfp.gpfn, &p2mt, P2M_ALLOC);
>>           if ( page )
>>           {
>>               guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
>>               put_page(page);
>>           }
>> +        else if ( p2m_is_foreign(p2mt) )
>> +            rc = p2m_remove_foreign(d, xrfp.gpfn);
>
> This doesn't seem like the right interface -- having special cases
> like this in the callers is something we slipped into in x86 for a lot
> of the paging/sharing code and it's not nice.  I think maybe we can
> just have get_page_from_gfn() DTRT for foreign (and grant) entries.
>
> Also, the comment will have been out of data by the time the x86
> version of this code is finished, as we won't be handling the refcount
> at this level. :)

I will remove the comment and modify get_page_from_gfn to handle foreign 
mapping.

-- 
Julien Grall