| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 3 Jun 2022 14:52:56 -0700 (PDT)
From: Stefano Stabellini <sstabellini@...nel.org>
To: Oleksandr Tyshchenko <olekstysh@...il.com>
cc: xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
Stefano Stabellini <sstabellini@...nel.org>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Juergen Gross <jgross@...e.com>, Julien Grall <julien@....org>
Subject: Re: [RFC PATCH 1/2] xen/unpopulated-alloc: Introduce helpers for
DMA allocations
On Tue, 17 May 2022, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>
>
> Add ability to allocate unpopulated DMAable (contiguous) pages
> suitable for grant mapping into. This is going to be used by gnttab
> code (see gnttab_dma_alloc_pages()).
>
> TODO: There is a code duplication in fill_dma_pool(). Also pool
> oparations likely need to be protected by the lock.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>
> ---
> drivers/xen/unpopulated-alloc.c | 167 ++++++++++++++++++++++++++++++++++++++++
> include/xen/xen.h | 15 ++++
> 2 files changed, 182 insertions(+)
>
> diff --git a/drivers/xen/unpopulated-alloc.c b/drivers/xen/unpopulated-alloc.c
> index a39f2d3..bca0198 100644
> --- a/drivers/xen/unpopulated-alloc.c
> +++ b/drivers/xen/unpopulated-alloc.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <linux/errno.h>
> +#include <linux/genalloc.h>
> #include <linux/gfp.h>
> #include <linux/kernel.h>
> #include <linux/mm.h>
> @@ -16,6 +17,8 @@ static DEFINE_MUTEX(list_lock);
> static struct page *page_list;
> static unsigned int list_count;
>
> +static struct gen_pool *dma_pool;
> +
> static struct resource *target_resource;
>
> /*
> @@ -230,6 +233,161 @@ void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages)
> }
> EXPORT_SYMBOL(xen_free_unpopulated_pages);
>
> +static int fill_dma_pool(unsigned int nr_pages)
> +{
I think we shouldn't need to add this function at all as we should be
able to reuse fill_list even for contiguous pages. fill_list could
always call gen_pool_add_virt before returning.
> + struct dev_pagemap *pgmap;
> + struct resource *res, *tmp_res = NULL;
> + void *vaddr;
> + unsigned int alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
> + struct range mhp_range;
> + int ret;
> +
> + res = kzalloc(sizeof(*res), GFP_KERNEL);
> + if (!res)
> + return -ENOMEM;
> +
> + res->name = "Xen DMA pool";
> + res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> +
> + mhp_range = mhp_get_pluggable_range(true);
> +
> + ret = allocate_resource(target_resource, res,
> + alloc_pages * PAGE_SIZE, mhp_range.start, mhp_range.end,
> + PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
> + if (ret < 0) {
> + pr_err("Cannot allocate new IOMEM resource\n");
> + goto err_resource;
> + }
> +
> + /*
> + * Reserve the region previously allocated from Xen resource to avoid
> + * re-using it by someone else.
> + */
> + if (target_resource != &iomem_resource) {
> + tmp_res = kzalloc(sizeof(*tmp_res), GFP_KERNEL);
> + if (!res) {
> + ret = -ENOMEM;
> + goto err_insert;
> + }
> +
> + tmp_res->name = res->name;
> + tmp_res->start = res->start;
> + tmp_res->end = res->end;
> + tmp_res->flags = res->flags;
> +
> + ret = request_resource(&iomem_resource, tmp_res);
> + if (ret < 0) {
> + pr_err("Cannot request resource %pR (%d)\n", tmp_res, ret);
> + kfree(tmp_res);
> + goto err_insert;
> + }
> + }
> +
> + pgmap = kzalloc(sizeof(*pgmap), GFP_KERNEL);
> + if (!pgmap) {
> + ret = -ENOMEM;
> + goto err_pgmap;
> + }
> +
> + pgmap->type = MEMORY_DEVICE_GENERIC;
> + pgmap->range = (struct range) {
> + .start = res->start,
> + .end = res->end,
> + };
> + pgmap->nr_range = 1;
> + pgmap->owner = res;
> +
> + vaddr = memremap_pages(pgmap, NUMA_NO_NODE);
> + if (IS_ERR(vaddr)) {
> + pr_err("Cannot remap memory range\n");
> + ret = PTR_ERR(vaddr);
> + goto err_memremap;
> + }
> +
> + ret = gen_pool_add_virt(dma_pool, (unsigned long)vaddr, res->start,
> + alloc_pages * PAGE_SIZE, NUMA_NO_NODE);
> + if (ret)
> + goto err_pool;
> +
> + return 0;
> +
> +err_pool:
> + memunmap_pages(pgmap);
> +err_memremap:
> + kfree(pgmap);
> +err_pgmap:
> + if (tmp_res) {
> + release_resource(tmp_res);
> + kfree(tmp_res);
> + }
> +err_insert:
> + release_resource(res);
> +err_resource:
> + kfree(res);
> + return ret;
> +}
> +
> +/**
> + * xen_alloc_unpopulated_dma_pages - alloc unpopulated DMAable pages
> + * @dev: valid struct device pointer
> + * @nr_pages: Number of pages
> + * @pages: pages returned
> + * @return 0 on success, error otherwise
> + */
> +int xen_alloc_unpopulated_dma_pages(struct device *dev, unsigned int nr_pages,
> + struct page **pages)
> +{
> + void *vaddr;
> + bool filled = false;
> + unsigned int i;
> + int ret;
Also probably it might be better if xen_alloc_unpopulated_pages and
xen_alloc_unpopulated_dma_pages shared the implementation. Something
along these lines:
int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages)
{
return _xen_alloc_unpopulated_pages(nr_pages, pages, false);
}
int xen_alloc_unpopulated_dma_pages(struct device *dev, unsigned int nr_pages,
struct page **pages)
{
return _xen_alloc_unpopulated_pages(nr_pages, pages, true);
}
static int _xen_alloc_unpopulated_pages(unsigned int nr_pages,
struct page **pages, bool contiguous)
{
unsigned int i;
int ret = 0;
if (contiguous && !xen_feature(XENFEAT_auto_translated_physmap))
return -EINVAL;
/*
* Fallback to default behavior if we do not have any suitable resource
* to allocate required region from and as the result we won't be able to
* construct pages.
*/
if (!target_resource) {
if (contiguous)
return -EINVAL;
return xen_alloc_ballooned_pages(nr_pages, pages);
}
mutex_lock(&list_lock);
if (list_count < nr_pages) {
ret = fill_list(nr_pages - list_count);
if (ret)
goto out;
}
if (contiguous) {
vaddr = (void *)gen_pool_alloc(dma_pool, nr_pages * PAGE_SIZE);
for (i = 0; i < nr_pages; i++)
pages[i] = virt_to_page(vaddr + PAGE_SIZE * i);
} else {
for (i = 0; i < nr_pages; i++) {
struct page *pg = page_list;
BUG_ON(!pg);
page_list = pg->zone_device_data;
list_count--;
pages[i] = pg;
#ifdef CONFIG_XEN_HAVE_PVMMU
if (!xen_feature(XENFEAT_auto_translated_physmap)) {
ret = xen_alloc_p2m_entry(page_to_pfn(pg));
if (ret < 0) {
unsigned int j;
for (j = 0; j <= i; j++) {
pages[j]->zone_device_data = page_list;
page_list = pages[j];
list_count++;
}
goto out;
}
}
#endif
}
}
out:
mutex_unlock(&list_lock);
return ret;
}
> + if (!dma_pool)
> + return -ENODEV;
> +
> + /* XXX Handle devices which support 64-bit DMA address only for now */
> + if (dma_get_mask(dev) != DMA_BIT_MASK(64))
> + return -EINVAL;
> +
> + while (!(vaddr = (void *)gen_pool_alloc(dma_pool, nr_pages * PAGE_SIZE))) {
> + if (filled)
> + return -ENOMEM;
> + else {
> + ret = fill_dma_pool(nr_pages);
> + if (ret)
> + return ret;
> +
> + filled = true;
> + }
> + }
> +
> + for (i = 0; i < nr_pages; i++)
> + pages[i] = virt_to_page(vaddr + PAGE_SIZE * i);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(xen_alloc_unpopulated_dma_pages);
> +
> +/**
> + * xen_free_unpopulated_dma_pages - return unpopulated DMAable pages
> + * @dev: valid struct device pointer
> + * @nr_pages: Number of pages
> + * @pages: pages to return
> + */
> +void xen_free_unpopulated_dma_pages(struct device *dev, unsigned int nr_pages,
> + struct page **pages)
> +{
> + void *vaddr;
> +
> + if (!dma_pool)
> + return;
> +
> + vaddr = page_to_virt(pages[0]);
> +
> + gen_pool_free(dma_pool, (unsigned long)vaddr, nr_pages * PAGE_SIZE);
> +}
> +EXPORT_SYMBOL(xen_free_unpopulated_dma_pages);
> +
> static int __init unpopulated_init(void)
> {
> int ret;
> @@ -241,8 +399,17 @@ static int __init unpopulated_init(void)
> if (ret) {
> pr_err("xen:unpopulated: Cannot initialize target resource\n");
> target_resource = NULL;
> + return ret;
> }
>
> + dma_pool = gen_pool_create(PAGE_SHIFT, NUMA_NO_NODE);
> + if (!dma_pool) {
> + pr_err("xen:unpopulated: Cannot create DMA pool\n");
> + return -ENOMEM;
> + }
> +
> + gen_pool_set_algo(dma_pool, gen_pool_best_fit, NULL);
> +
> return ret;
> }
> early_initcall(unpopulated_init);
> diff --git a/include/xen/xen.h b/include/xen/xen.h
> index a99bab8..a6a7a59 100644
> --- a/include/xen/xen.h
> +++ b/include/xen/xen.h
> @@ -52,9 +52,15 @@ bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
> extern u64 xen_saved_max_mem_size;
> #endif
>
> +struct device;
> +
> #ifdef CONFIG_XEN_UNPOPULATED_ALLOC
> int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages);
> void xen_free_unpopulated_pages(unsigned int nr_pages, struct page **pages);
> +int xen_alloc_unpopulated_dma_pages(struct device *dev, unsigned int nr_pages,
> + struct page **pages);
> +void xen_free_unpopulated_dma_pages(struct device *dev, unsigned int nr_pages,
> + struct page **pages);
> #include <linux/ioport.h>
> int arch_xen_unpopulated_init(struct resource **res);
> #else
> @@ -69,6 +75,15 @@ static inline void xen_free_unpopulated_pages(unsigned int nr_pages,
> {
> xen_free_ballooned_pages(nr_pages, pages);
> }
> +static inline int xen_alloc_unpopulated_dma_pages(struct device *dev,
> + unsigned int nr_pages, struct page **pages)
> +{
> + return -1;
> +}
> +static inline void xen_free_unpopulated_dma_pages(struct device *dev,
> + unsigned int nr_pages, struct page **pages)
> +{
> +}
> #endif
Given that we have these stubs, maybe we don't need to #ifdef the so
much code in the next patch
Powered by blists - more mailing lists