Re: [PATCH v6 08/10] xen/arm: Add relinquish_p2m_mapping to remove reference on every mapped page

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

On 12/17/2013 04:36 PM, Ian Campbell wrote:
> On Tue, 2013-12-17 at 16:27 +0000, Julien Grall wrote:
>> This function will be called when the domain relinquishes its memory.
>> It removes refcount on every mapped page to a valid MFN.
>>
>> Currently, Xen doesn't take reference on every new mapping but only for foreign
>> mapping. Restrict the function only on foreign mapping.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> ---
>>     Changes in v6:
>>         - Typoes
>>         - Rework preempt
>>         - Clean up if ( p2mt == ... || p2mt == ... )
> 
> You don't seem to have addressed the issue I pointed out about
> create_p2m_entries exiting on any holes which it finds at the first or
> second level.
> 
> I think you missed my second reply to 08/10 on v5 see:
> <1387279904.27441.47.camel@kazak.uk.xensource.com>

Sorry I forgot to address your comment (see the copy of your email here):

> If this function finds any non-present first or second level PTE then it
> will stop and exit (goto out), meaning it will miss any mappings which
> are higher up after the hole.
>
> e.g. if you have a guest p2m with RAM at 0-2M and 4-6M then
> relinquishing 0-6M will only actually free 0-2M, then abort on 4-6M.
>
> Perhaps this could be fixed by making relinquish_p2m_mapping loop over
> the address space relinquishing 2M chunks as it goes? This would
remove
> the need for the if ( op == RELINQUISH && .. && prempt() ) stuff,
> because you could add the preempt in that loop.

I think you are wrong:
	- every first pte exists. We return NULL in case of the address is very
high.
        - if the second pte doesn't exist, we will create it (if it
fails we go out).

For the last item, I think it's a bit stupid to create table if we are
removing/relinquish mapping. But I think it's an improvement for later.
There are lots of improvement to do in this function (eg: flushing).

-- 
Julien Grall