Re: [PATCH v2 08/10] xen/arm: Implement xen_rem_foreign_from_p2m

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

On 12/09/2013 04:31 PM, Ian Campbell wrote:
> On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote:
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> ---
>>     Changes in v2:
>>         - Introduce the patch
>> ---
>>  xen/arch/arm/mm.c         |   16 ++++++++++++++++
>>  xen/include/asm-arm/p2m.h |    6 +-----
>>  2 files changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index 960c872..ba51f6e 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -968,6 +968,22 @@ void share_xen_page_with_privileged_guests(
>>      share_xen_page_with_guest(page, dom_xen, readonly);
>>  }
>>  
>> +int xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn)
>> +{
>> +    unsigned long mfn = gmfn_to_mfn(d, gpfn);
>> +    if ( !mfn_valid(mfn) )
>> +    {
>> +        gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx domid:%d\n",
>> +                 gpfn, d->domain_id);
> 
> Is this guest triggerable? Or does it indicate a mistake when we made
> the mapping?

It indicates a mistake when we made the mapping. Perhaps an ASSERT is
better here.

> 
>> +        return -EINVAL;
>> +    }
>> +
>> +    guest_physmap_remove_page(d, gpfn, mfn, 0);
>> +    put_page(mfn_to_page(mfn));
> 
> Some of this could become a common helper with create_p2m_entries and
> the relinquish stuff in the previous patch.

The best solution is to move put_page in guest_physmap_remove_page. But
as we didn't plan to handle refcount for each mapping in Xen 4.4,
perhaps we can delay it?

-- 
Julien Grall