Re: [RFC 01/19] xen/arm: guest_physmap_remove_page: Print a warning if we fail to unmap the page

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

Hi Ian,

On 07/03/2014 11:52 AM, Ian Campbell wrote:
> On Mon, 2014-06-16 at 17:17 +0100, Julien Grall wrote:
>> The function guest_physmap_remove_page does't have a return value. With
>> the change "arch/arm: add consistency check to REMOVE p2m changes",
>> apply_p2m_changes can unlikely fail.
> 
> Looking at v9 of that patch I don't see it adding any new failures, is
> this comment (I suppose written against an older version) still
> accurate?

It doesn't look like accurate for v9. I can change the commit message to
explain that apply_p2m_changes may fail. So it's better to log it/

>>  Warn the user in this case.
> 
> Given that apply_p2m_changes can fail it seems reasonable to log
> regardless of the above:
>         Acked-by: Ian Campbell <ian.campbell@citrix.com>
> Two questions:
> 
> Would it be better to instead ensure that apply_p2m_changes always logs
> on failure? I suppose it would be more able to give a specific message.

I didn't though about this solution. It may be easier for debugging later.


> On failure do we retain any reference counts to prevent these pages
> getting reused?

We don't have any mapping reference count so it's fine.

I've asked few changes in Arianna's series to clear the p2m even if the
mapping doesn't correspond. Otherwise we may have issue with foreign
mapping.

I will drop this patch from the series and send a separate patch.

Regards,

-- 
Julien Grall