Re: [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed

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

On 12/19/2013 09:38 AM, Ian Campbell wrote:
> On Wed, 2013-12-18 at 17:31 +0000, Julien Grall wrote:
>> When  Xen is removing/relinquishing mapping, it will create second/third tables
>> if they don't exist.
>>
>> Non-existent table means the address range was never mapped, so Xen can safely
>> skip them.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>
>> ---
>>     Release: This is an improvement for Xen 4.4. It will save time during
>>     relinquish phase and avoid dummy allocation.
>>     The downside is the patch is modifying p2m loop which is used everywhere.
>> ---
>>  xen/arch/arm/p2m.c |   22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>> index d24a6fc..9ef8819 100644
>> --- a/xen/arch/arm/p2m.c
>> +++ b/xen/arch/arm/p2m.c
>> @@ -246,10 +246,12 @@ static int create_p2m_entries(struct domain *d,
>>                    cur_first_offset = ~0,
>>                    cur_second_offset = ~0;
>>      unsigned long count = 0;
>> +    bool_t populate = (op == INSERT || op == ALLOCATE);
>>  
>>      spin_lock(&p2m->lock);
>>  
>> -    for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE)
>> +    addr = start_gpaddr;
>> +    while ( addr < end_gpaddr )
>>      {
>>          if ( cur_first_page != p2m_first_level_index(addr) )
>>          {
>> @@ -265,8 +267,15 @@ static int create_p2m_entries(struct domain *d,
>>  
>>          if ( !first[first_table_offset(addr)].p2m.valid )
>>          {
>> +            if ( !populate )
>> +            {
>> +                addr += FIRST_SIZE;
> 
> I think this subtly does the wrong thing if addr is not FIRST_SIZE
> aligned, which it might be on the first iteration. That will skip the
> start of the next 1st level block.
> 
> I think addr needs to be rounded up to the next first boundary.

Right, I will send a new version of this patch.

-- 
Julien Grall