Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 29 Jun 2022 18:06:57 -0700 (PDT)
From:   Stefano Stabellini <sstabellini@kernel.org>
To:     Oleksandr Tyshchenko <olekstysh@gmail.com>
cc:     xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Juergen Gross <jgross@suse.com>, Julien Grall <julien@xen.org>
Subject: Re: [PATCH V1 2/2] xen/grant-table: Use unpopulated contiguous pages
 instead of real RAM ones
In-Reply-To: <1655740136-3974-3-git-send-email-olekstysh@gmail.com>
Message-ID: <alpine.DEB.2.22.394.2206291756110.4389@ubuntu-linux-20-04-desktop>
References: <1655740136-3974-1-git-send-email-olekstysh@gmail.com> <1655740136-3974-3-git-send-email-olekstysh@gmail.com>
User-Agent: Alpine 2.22 (DEB 394 2020-01-19)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Mon, 20 Jun 2022, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> Depends on CONFIG_XEN_UNPOPULATED_ALLOC. If enabled then unpopulated
> contiguous pages will be allocated for grant mapping into instead of
> ballooning out real RAM pages.
> 
> Also fallback to allocate DMAable pages (balloon out real RAM pages)
> if we failed to allocate unpopulated contiguous pages. Use recently
> introduced is_xen_unpopulated_page() in gnttab_dma_free_pages() to know
> what API to use for freeing pages.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
> Please note, I haven't re-checked yet the use-case where the xen-swiotlb
> is involved (proposed by Stefano):
> https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2206031348230.2783803@ubuntu-linux-20-04-desktop/
> I will re-check that for next version and add corresponding comment
> in the code.

Great. The patch looks good so far.


> Changes RFC -> V1:
>    - update commit subject/description
>    - rework to avoid introducing alternative implementation
>      of gnttab_dma_alloc(free)_pages(), use IS_ENABLED()
>    - implement a fallback to real RAM pages if we failed to allocate
>      unpopulated contiguous pages (resolve initial TODO)
>    - update according to the API renaming (s/dma/contiguous)
> ---
>  drivers/xen/grant-table.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 738029d..15e426b 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -1047,6 +1047,23 @@ int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args)
>  	size_t size;
>  	int i, ret;
>  
> +	if (IS_ENABLED(CONFIG_XEN_UNPOPULATED_ALLOC)) {
> +		ret = xen_alloc_unpopulated_contiguous_pages(args->dev, args->nr_pages,
> +				args->pages);
> +		if (ret < 0)
> +			goto fallback;
> +
> +		ret = gnttab_pages_set_private(args->nr_pages, args->pages);
> +		if (ret < 0)
> +			goto fail;
> +
> +		args->vaddr = page_to_virt(args->pages[0]);
> +		args->dev_bus_addr = page_to_phys(args->pages[0]);
> +
> +		return ret;
> +	}
> +
> +fallback:
>  	size = args->nr_pages << PAGE_SHIFT;
>  	if (args->coherent)
>  		args->vaddr = dma_alloc_coherent(args->dev, size,
> @@ -1103,6 +1120,13 @@ int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args)
>  
>  	gnttab_pages_clear_private(args->nr_pages, args->pages);
>  
> +	if (IS_ENABLED(CONFIG_XEN_UNPOPULATED_ALLOC) &&
> +			is_xen_unpopulated_page(args->pages[0])) {
> +		xen_free_unpopulated_contiguous_pages(args->dev, args->nr_pages,
> +				args->pages);
> +		return 0;
> +	}
> +
>  	for (i = 0; i < args->nr_pages; i++)
>  		args->frames[i] = page_to_xen_pfn(args->pages[i]);
>  
> -- 
> 2.7.4
>