Re: [RFC v2 2/6] xen/arm: implement support for XENMEM_maximum_gpfn hypercall

[Wei Huang <w1.huang@samsung.com>]

On 04/15/2014 05:46 PM, Julien Grall wrote:
> Hello Wei,
>
> Thank you for the patch.
>
> On 15/04/14 22:05, Wei Huang wrote:
>> From: Jaeyong Yoo <jaeyong.yoo@samsung.com>
>>
>> This patch implements ddomain_get_maximum_gpfn by using max_mapped_gfn
>> field of P2M struct. A support function to retrieve guest VM pfn range
>> is also added.
>>
>> Signed-off-by: Evgeny Fedotov <e.fedotov@samsung.com>
>> Signed-off-by: Wei Huang <w1.huang@samsung.com>
>> ---
>>   xen/arch/arm/mm.c        |   21 ++++++++++++++++++++-
>>   xen/include/asm-arm/mm.h |    1 +
>>   2 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index 362bc8d..473ad04 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -947,7 +947,11 @@ int page_is_ram_type(unsigned long mfn, unsigned
>> long mem_type)
>>
>>   unsigned long domain_get_maximum_gpfn(struct domain *d)
>>   {
>> -    return -ENOSYS;
>> +    paddr_t end;
>> +
>> +    domain_get_gpfn_range(d, NULL, &end);
>> +
>> +    return (unsigned long)end;
>>   }
>>
>>   void share_xen_page_with_guest(struct page_info *page,
>> @@ -1235,6 +1239,21 @@ int is_iomem_page(unsigned long mfn)
>>           return 1;
>>       return 0;
>>   }
>> +
>> +/*
>> + * Return start and end addresses of guest VM
>> + */
>> +void domain_get_gpfn_range(struct domain *d, paddr_t *start, paddr_t
>> *end)
>
> The content of the function doesn't match the name.
>
> This function should return a PFN not an address.
> Actually, libxc (i.e the return of domain_get_maximum_gpfn) expect a pfn.
>
>> +{
>> +    struct p2m_domain *p2m = &d->arch.p2m;
>> +
>> +    if ( start )
>> +        *start = GUEST_RAM_BASE;
>
> You can use  p2m->lowest_mapped_gfn here.
>
>> +    if ( end )
>> +        *end = GUEST_RAM_BASE + ((paddr_t) p2m->max_mapped_gfn);
>
> This is wrong, max_mapped_gpfn contains a guest frame number, not a
> number of frames.
>
> The code should be smth like:
>
> *end = pfn_to_paddr(p2m->max_mapped_gfn);
I will correct them. This patch set touches too many parts. I will break 
them into smaller patches to satisfy the comments.
>
> Regards,
>