Re: [PATCH 6/8] xen/arm: Retrieve p2m type in get_page_from_gfn

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

On 12/05/2013 04:23 PM, Ian Campbell wrote:
> On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote:
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>   xen/include/asm-arm/p2m.h |    5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
>> index 3de69c4..54d1dab 100644
>> --- a/xen/include/asm-arm/p2m.h
>> +++ b/xen/include/asm-arm/p2m.h
>> @@ -105,9 +105,8 @@ static inline struct page_info *get_page_from_gfn(
>>       struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
>>   {
>>       struct page_info *page;
>> -    unsigned long mfn = gmfn_to_mfn(d, gfn);
>> -
>> -    ASSERT(t == NULL);
>> +    paddr_t maddr = p2m_get_entry(d, pfn_to_paddr(gfn), t);
>> +    unsigned long mfn = maddr >> PAGE_SHIFT;
>
> Do we need to eg. convert p2m_invalid into returning NULL instead of
> whatever maddr contains in that case?

In that case maddr will contain INVALID_PADDR, which will turn in 
INVALID_MFN with the shift. So we don't need to check the p2m type.

>
> In the case of p2m_mmio_direct we need to be careful because there is no
> struct page. Ah.. here it is in the context:
>>       if (!mfn_valid(mfn))
>>           return NULL;
>
> Although I wonder if we should convert mmio_direct into NULL even if the
> MMIO region happens to have a struct page? (e.g. due to holes in the
> frametable)

Actually, I think it's fine. get_page will fail because the page won't 
belong to the domain. So the function will return NULL.

-- 
Julien Grall