Re: [Xen-devel] [PATCH 02/13] xen/grant-table: Move xlated_setup_gnttab_pages to common place

[Shannon Zhao <zhaoshenglong@xxxxxxxxxx>]

On 2015/11/18 0:02, David Vrabel wrote:
> On 17/11/15 09:57, shannon.zhao@xxxxxxxxxx wrote:
>> From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>>
>> Move xlated_setup_gnttab_pages to common place, so it can be reused by
>> ARM to setup grant table when booting with ACPI.
> [...]
>> --- a/drivers/xen/grant-table.c
>> +++ b/drivers/xen/grant-table.c
>> @@ -664,6 +664,55 @@ int gnttab_setup_auto_xlat_frames(phys_addr_t addr)
>>  }
>>  EXPORT_SYMBOL_GPL(gnttab_setup_auto_xlat_frames);
>>  
>> +int __init xlated_setup_gnttab_pages(void)
>> +{
>> +    struct page **pages;
>> +    xen_pfn_t *pfns;
>> +    void *vaddr;
>> +    int rc;
>> +    unsigned int i;
>> +    unsigned long nr_grant_frames = gnttab_max_grant_frames();
>> +
>> +    BUG_ON(nr_grant_frames == 0);
>> +    pages = kcalloc(nr_grant_frames, sizeof(pages[0]), GFP_KERNEL);
>> +    if (!pages)
>> +            return -ENOMEM;
>> +
>> +    pfns = kcalloc(nr_grant_frames, sizeof(pfns[0]), GFP_KERNEL);
>> +    if (!pfns) {
>> +            kfree(pages);
>> +            return -ENOMEM;
>> +    }
>> +    rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
>> +    if (rc) {
>> +            pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
>> +                    nr_grant_frames, rc);
>> +            kfree(pages);
>> +            kfree(pfns);
>> +            return rc;
>> +    }
>> +    for (i = 0; i < nr_grant_frames; i++)
>> +            pfns[i] = page_to_pfn(pages[i]);
>> +
>> +    vaddr = vmap(pages, nr_grant_frames, 0, PAGE_KERNEL);
>> +    if (!vaddr) {
>> +            pr_warn("%s Couldn't map %ld pfns rc:%d\n", __func__,
>> +                    nr_grant_frames, rc);
>> +            free_xenballooned_pages(nr_grant_frames, pages);
>> +            kfree(pages);
>> +            kfree(pfns);
>> +            return -ENOMEM;
>> +    }
>> +    kfree(pages);
> 
> Can you move most of this function into drivers/xen/xlate_mmu.c in a
> function like:
> 
> /**
>  * xen_xlate_map_ballooned_pages - map a new set of ballooned pages
>  * count: number of GFNs.
>  * pages: returns the array of (now) ballooned pages.
>  * gfns: returns the array of corresponding GFNs.
>  * virt: returns the virtual address of the mapped region.
>  *
>  * This allocate a set of ballooned pages and maps them into the
>  * kernel's address space.
>  */
> void *xen_xlate_map_ballooned_pages(
>     unsigned int count,
>     struct page **pages,
>     xen_pfn_t **gfns);
> 
Sure, but the parameter "pages" is not necessary I think, since the
pages has been freed in the function and it doesn't need in
xen_auto_xlat_grant_frames either. Right?

Thanks,
-- 
Shannon


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel