Re: [RFC PATCH 1/2] xen/unpopulated-alloc: Introduce helpers for DMA allocations
On Tue, Jun 14, 2022 at 05:45:53PM -0700, Stefano Stabellini wrote: > Basically, if we allocate (and free) page-by-page it leads to more > efficient resource utilization but it is slower. If we allocate larger > contiguous chunks it is faster but it leads to less efficient resource > utilization. > > Given that on both x86 and ARM the unpopulated memory resource is > arbitrarily large, I don't think we need to worry about resource > utilization. It is not backed by real memory. The only limitation is the > address space size which is very large. Well, unpopulated memory will consume memory to populate the related metadata (ie: struct page and realted tracking information) for the unpopulated region, so it's not completely free. Thanks, Roger.
Re: [RFC PATCH 1/2] xen/unpopulated-alloc: Introduce helpers for DMA allocations
On 15.06.22 03:45, Stefano Stabellini wrote:
Hello Stefano
On Tue, 14 Jun 2022, Oleksandr wrote:
On 11.06.22 03:12, Stefano Stabellini wrote:
On Wed, 8 Jun 2022, Oleksandr wrote:
2. Drop the "page_list" entirely and use "dma_pool" for all (contiguous
and
non-contiguous) allocations. After all, all pages are initially contiguous
in
fill_list() as they are built from the resource. This changes behavior for
all
users of xen_alloc_unpopulated_pages()
Below the diff for unpopulated-alloc.c. The patch is also available at:
https://github.com/otyshchenko1/linux/commit/7be569f113a4acbdc4bcb9b20cb3995b3151387a
diff --git a/drivers/xen/unpopulated-alloc.c
b/drivers/xen/unpopulated-alloc.c
index a39f2d3..ab5c7bd 100644
--- a/drivers/xen/unpopulated-alloc.c
+++ b/drivers/xen/unpopulated-alloc.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
+#include
#include
+#include
#include
#include
#include
@@ -13,8 +15,8 @@
#include
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;
@@ -36,7 +38,7 @@ static int fill_list(unsigned int nr_pages)
struct dev_pagemap *pgmap;
struct resource *res, *tmp_res = NULL;
void *vaddr;
- unsigned int i, alloc_pages = round_up(nr_pages,
PAGES_PER_SECTION);
+ unsigned int alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
struct range mhp_range;
int ret;
@@ -106,6 +108,7 @@ static int fill_list(unsigned int nr_pages)
* conflict with any devices.
*/
if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+ unsigned int i;
xen_pfn_t pfn = PFN_DOWN(res->start);
for (i = 0; i < alloc_pages; i++) {
@@ -125,16 +128,17 @@ static int fill_list(unsigned int nr_pages)
goto err_memremap;
}
- for (i = 0; i < alloc_pages; i++) {
- struct page *pg = virt_to_page(vaddr + PAGE_SIZE * i);
-
- pg->zone_device_data = page_list;
- page_list = pg;
- list_count++;
+ ret = gen_pool_add_virt(dma_pool, (unsigned long)vaddr,
res->start,
+ alloc_pages * PAGE_SIZE, NUMA_NO_NODE);
+ if (ret) {
+ pr_err("Cannot add memory range to the pool\n");
+ goto err_pool;
}
return 0;
+err_pool:
+ memunmap_pages(pgmap);
err_memremap:
kfree(pgmap);
err_pgmap:
@@ -149,51 +153,49 @@ static int fill_list(unsigned int nr_pages)
return ret;
}
-/**
- * xen_alloc_unpopulated_pages - alloc unpopulated pages
- * @nr_pages: Number of pages
- * @pages: pages returned
- * @return 0 on success, error otherwise
- */
-int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page
**pages)
+static int alloc_unpopulated_pages(unsigned int nr_pages, struct page
**pages,
+ bool contiguous)
{
unsigned int i;
int ret = 0;
+ void *vaddr;
+ bool filled = false;
/*
* 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 (!target_resource) {
+ if (contiguous)
+ return -ENODEV;
+
return xen_alloc_ballooned_pages(nr_pages, pages);
+ }
mutex_lock(_lock);
- if (list_count < nr_pages) {
- ret = fill_list(nr_pages - list_count);
+
+ while (!(vaddr = (void *)gen_pool_alloc(dma_pool, nr_pages *
PAGE_SIZE))) {
+ if (filled)
+ ret = -ENOMEM;
+ else {
+ ret = fill_list(nr_pages);
+ filled = true;
+ }
if (ret)
goto out;
}
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;
+ pages[i] = virt_to_page(vaddr + PAGE_SIZE * i);
#ifdef CONFIG_XEN_HAVE_PVMMU
if (!xen_feature(XENFEAT_auto_translated_physmap)) {
- ret = xen_alloc_p2m_entry(page_to_pfn(pg));
+ ret = xen_alloc_p2m_entry(page_to_pfn(pages[i]));
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++;
- }
+ /* XXX Do we need to
Re: [RFC PATCH 1/2] xen/unpopulated-alloc: Introduce helpers for DMA allocations
On Tue, 14 Jun 2022, Oleksandr wrote:
> On 11.06.22 03:12, Stefano Stabellini wrote:
> > On Wed, 8 Jun 2022, Oleksandr wrote:
> > > 2. Drop the "page_list" entirely and use "dma_pool" for all (contiguous
> > > and
> > > non-contiguous) allocations. After all, all pages are initially contiguous
> > > in
> > > fill_list() as they are built from the resource. This changes behavior for
> > > all
> > > users of xen_alloc_unpopulated_pages()
> > >
> > > Below the diff for unpopulated-alloc.c. The patch is also available at:
> > >
> > > https://github.com/otyshchenko1/linux/commit/7be569f113a4acbdc4bcb9b20cb3995b3151387a
> > >
> > >
> > > diff --git a/drivers/xen/unpopulated-alloc.c
> > > b/drivers/xen/unpopulated-alloc.c
> > > index a39f2d3..ab5c7bd 100644
> > > --- a/drivers/xen/unpopulated-alloc.c
> > > +++ b/drivers/xen/unpopulated-alloc.c
> > > @@ -1,5 +1,7 @@
> > > // SPDX-License-Identifier: GPL-2.0
> > > +#include
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -13,8 +15,8 @@
> > > #include
> > >
> > > 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;
> > >
> > > @@ -36,7 +38,7 @@ static int fill_list(unsigned int nr_pages)
> > > struct dev_pagemap *pgmap;
> > > struct resource *res, *tmp_res = NULL;
> > > void *vaddr;
> > > - unsigned int i, alloc_pages = round_up(nr_pages,
> > > PAGES_PER_SECTION);
> > > + unsigned int alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
> > > struct range mhp_range;
> > > int ret;
> > >
> > > @@ -106,6 +108,7 @@ static int fill_list(unsigned int nr_pages)
> > > * conflict with any devices.
> > > */
> > > if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> > > + unsigned int i;
> > > xen_pfn_t pfn = PFN_DOWN(res->start);
> > >
> > > for (i = 0; i < alloc_pages; i++) {
> > > @@ -125,16 +128,17 @@ static int fill_list(unsigned int nr_pages)
> > > goto err_memremap;
> > > }
> > >
> > > - for (i = 0; i < alloc_pages; i++) {
> > > - struct page *pg = virt_to_page(vaddr + PAGE_SIZE * i);
> > > -
> > > - pg->zone_device_data = page_list;
> > > - page_list = pg;
> > > - list_count++;
> > > + ret = gen_pool_add_virt(dma_pool, (unsigned long)vaddr,
> > > res->start,
> > > + alloc_pages * PAGE_SIZE, NUMA_NO_NODE);
> > > + if (ret) {
> > > + pr_err("Cannot add memory range to the pool\n");
> > > + goto err_pool;
> > > }
> > >
> > > return 0;
> > >
> > > +err_pool:
> > > + memunmap_pages(pgmap);
> > > err_memremap:
> > > kfree(pgmap);
> > > err_pgmap:
> > > @@ -149,51 +153,49 @@ static int fill_list(unsigned int nr_pages)
> > > return ret;
> > > }
> > >
> > > -/**
> > > - * xen_alloc_unpopulated_pages - alloc unpopulated pages
> > > - * @nr_pages: Number of pages
> > > - * @pages: pages returned
> > > - * @return 0 on success, error otherwise
> > > - */
> > > -int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page
> > > **pages)
> > > +static int alloc_unpopulated_pages(unsigned int nr_pages, struct page
> > > **pages,
> > > + bool contiguous)
> > > {
> > > unsigned int i;
> > > int ret = 0;
> > > + void *vaddr;
> > > + bool filled = false;
> > >
> > > /*
> > > * 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 (!target_resource) {
> > > + if (contiguous)
> > > + return -ENODEV;
> > > +
> > > return xen_alloc_ballooned_pages(nr_pages, pages);
> > > + }
> > >
> > > mutex_lock(_lock);
> > > - if (list_count < nr_pages) {
> > > - ret = fill_list(nr_pages - list_count);
> > > +
> > > + while (!(vaddr = (void *)gen_pool_alloc(dma_pool, nr_pages *
> > > PAGE_SIZE))) {
> > > + if (filled)
> > > + ret = -ENOMEM;
> > > + else {
> > > + ret = fill_list(nr_pages);
> > > + filled = true;
> > > + }
> > > if (ret)
> > > goto out;
> > > }
> > >
> > > for (i = 0; i < nr_pages; i++) {
> > > - struct page *pg = page_list;
> > > -
> > > - BUG_ON(!pg);
> > > - page_list = pg->zone_device_data;
> > > - list_count--;
> > >
Re: [RFC PATCH 1/2] xen/unpopulated-alloc: Introduce helpers for DMA allocations
On 11.06.22 03:12, Stefano Stabellini wrote:
Hello Stefano
On Wed, 8 Jun 2022, Oleksandr wrote:
2. Drop the "page_list" entirely and use "dma_pool" for all (contiguous and
non-contiguous) allocations. After all, all pages are initially contiguous in
fill_list() as they are built from the resource. This changes behavior for all
users of xen_alloc_unpopulated_pages()
Below the diff for unpopulated-alloc.c. The patch is also available at:
https://github.com/otyshchenko1/linux/commit/7be569f113a4acbdc4bcb9b20cb3995b3151387a
diff --git a/drivers/xen/unpopulated-alloc.c b/drivers/xen/unpopulated-alloc.c
index a39f2d3..ab5c7bd 100644
--- a/drivers/xen/unpopulated-alloc.c
+++ b/drivers/xen/unpopulated-alloc.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
+#include
#include
+#include
#include
#include
#include
@@ -13,8 +15,8 @@
#include
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;
@@ -36,7 +38,7 @@ static int fill_list(unsigned int nr_pages)
struct dev_pagemap *pgmap;
struct resource *res, *tmp_res = NULL;
void *vaddr;
- unsigned int i, alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
+ unsigned int alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
struct range mhp_range;
int ret;
@@ -106,6 +108,7 @@ static int fill_list(unsigned int nr_pages)
* conflict with any devices.
*/
if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+ unsigned int i;
xen_pfn_t pfn = PFN_DOWN(res->start);
for (i = 0; i < alloc_pages; i++) {
@@ -125,16 +128,17 @@ static int fill_list(unsigned int nr_pages)
goto err_memremap;
}
- for (i = 0; i < alloc_pages; i++) {
- struct page *pg = virt_to_page(vaddr + PAGE_SIZE * i);
-
- pg->zone_device_data = page_list;
- page_list = pg;
- list_count++;
+ ret = gen_pool_add_virt(dma_pool, (unsigned long)vaddr, res->start,
+ alloc_pages * PAGE_SIZE, NUMA_NO_NODE);
+ if (ret) {
+ pr_err("Cannot add memory range to the pool\n");
+ goto err_pool;
}
return 0;
+err_pool:
+ memunmap_pages(pgmap);
err_memremap:
kfree(pgmap);
err_pgmap:
@@ -149,51 +153,49 @@ static int fill_list(unsigned int nr_pages)
return ret;
}
-/**
- * xen_alloc_unpopulated_pages - alloc unpopulated pages
- * @nr_pages: Number of pages
- * @pages: pages returned
- * @return 0 on success, error otherwise
- */
-int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages)
+static int alloc_unpopulated_pages(unsigned int nr_pages, struct page
**pages,
+ bool contiguous)
{
unsigned int i;
int ret = 0;
+ void *vaddr;
+ bool filled = false;
/*
* 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 (!target_resource) {
+ if (contiguous)
+ return -ENODEV;
+
return xen_alloc_ballooned_pages(nr_pages, pages);
+ }
mutex_lock(_lock);
- if (list_count < nr_pages) {
- ret = fill_list(nr_pages - list_count);
+
+ while (!(vaddr = (void *)gen_pool_alloc(dma_pool, nr_pages *
PAGE_SIZE))) {
+ if (filled)
+ ret = -ENOMEM;
+ else {
+ ret = fill_list(nr_pages);
+ filled = true;
+ }
if (ret)
goto out;
}
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;
+ pages[i] = virt_to_page(vaddr + PAGE_SIZE * i);
#ifdef CONFIG_XEN_HAVE_PVMMU
if (!xen_feature(XENFEAT_auto_translated_physmap)) {
- ret = xen_alloc_p2m_entry(page_to_pfn(pg));
+ ret = xen_alloc_p2m_entry(page_to_pfn(pages[i]));
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++;
- }
+ /* XXX Do we need to zeroed pages[i]? */
+ gen_pool_free(dma_pool, (unsigned long)vaddr,
+
Re: [RFC PATCH 1/2] xen/unpopulated-alloc: Introduce helpers for DMA allocations
On Wed, 8 Jun 2022, Oleksandr wrote:
> 2. Drop the "page_list" entirely and use "dma_pool" for all (contiguous and
> non-contiguous) allocations. After all, all pages are initially contiguous in
> fill_list() as they are built from the resource. This changes behavior for all
> users of xen_alloc_unpopulated_pages()
>
> Below the diff for unpopulated-alloc.c. The patch is also available at:
>
> https://github.com/otyshchenko1/linux/commit/7be569f113a4acbdc4bcb9b20cb3995b3151387a
>
>
> diff --git a/drivers/xen/unpopulated-alloc.c b/drivers/xen/unpopulated-alloc.c
> index a39f2d3..ab5c7bd 100644
> --- a/drivers/xen/unpopulated-alloc.c
> +++ b/drivers/xen/unpopulated-alloc.c
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> +#include
> #include
> +#include
> #include
> #include
> #include
> @@ -13,8 +15,8 @@
> #include
>
> 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;
>
> @@ -36,7 +38,7 @@ static int fill_list(unsigned int nr_pages)
> struct dev_pagemap *pgmap;
> struct resource *res, *tmp_res = NULL;
> void *vaddr;
> - unsigned int i, alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
> + unsigned int alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
> struct range mhp_range;
> int ret;
>
> @@ -106,6 +108,7 @@ static int fill_list(unsigned int nr_pages)
> * conflict with any devices.
> */
> if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> + unsigned int i;
> xen_pfn_t pfn = PFN_DOWN(res->start);
>
> for (i = 0; i < alloc_pages; i++) {
> @@ -125,16 +128,17 @@ static int fill_list(unsigned int nr_pages)
> goto err_memremap;
> }
>
> - for (i = 0; i < alloc_pages; i++) {
> - struct page *pg = virt_to_page(vaddr + PAGE_SIZE * i);
> -
> - pg->zone_device_data = page_list;
> - page_list = pg;
> - list_count++;
> + ret = gen_pool_add_virt(dma_pool, (unsigned long)vaddr, res->start,
> + alloc_pages * PAGE_SIZE, NUMA_NO_NODE);
> + if (ret) {
> + pr_err("Cannot add memory range to the pool\n");
> + goto err_pool;
> }
>
> return 0;
>
> +err_pool:
> + memunmap_pages(pgmap);
> err_memremap:
> kfree(pgmap);
> err_pgmap:
> @@ -149,51 +153,49 @@ static int fill_list(unsigned int nr_pages)
> return ret;
> }
>
> -/**
> - * xen_alloc_unpopulated_pages - alloc unpopulated pages
> - * @nr_pages: Number of pages
> - * @pages: pages returned
> - * @return 0 on success, error otherwise
> - */
> -int xen_alloc_unpopulated_pages(unsigned int nr_pages, struct page **pages)
> +static int alloc_unpopulated_pages(unsigned int nr_pages, struct page
> **pages,
> + bool contiguous)
> {
> unsigned int i;
> int ret = 0;
> + void *vaddr;
> + bool filled = false;
>
> /*
> * 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 (!target_resource) {
> + if (contiguous)
> + return -ENODEV;
> +
> return xen_alloc_ballooned_pages(nr_pages, pages);
> + }
>
> mutex_lock(_lock);
> - if (list_count < nr_pages) {
> - ret = fill_list(nr_pages - list_count);
> +
> + while (!(vaddr = (void *)gen_pool_alloc(dma_pool, nr_pages *
> PAGE_SIZE))) {
> + if (filled)
> + ret = -ENOMEM;
> + else {
> + ret = fill_list(nr_pages);
> + filled = true;
> + }
> if (ret)
> goto out;
> }
>
> 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;
> + pages[i] = virt_to_page(vaddr + PAGE_SIZE * i);
>
> #ifdef CONFIG_XEN_HAVE_PVMMU
> if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> - ret = xen_alloc_p2m_entry(page_to_pfn(pg));
> + ret = xen_alloc_p2m_entry(page_to_pfn(pages[i]));
> 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++;
> - }
>
Re: [RFC PATCH 1/2] xen/unpopulated-alloc: Introduce helpers for DMA allocations
On 04.06.22 00:52, Stefano Stabellini wrote:
Hello Stefano
Thank you for having a look and sorry for the late response.
On Tue, 17 May 2022, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko
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
---
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
+#include
#include
#include
#include
@@ -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.
First of all, I agree that fill_dma_pool() has a lot in common with
fill_list(), so we indeed can avoid code duplication (this was mentioned
in TODO).
I am not quite sure regarding "to always call gen_pool_add_virt before
returning" (does this mean that the same pages will be in the
"page_list" and "dma_pool" simultaneously?),
but I completely agree that we can reuse fill_list() for contiguous
pages as well slightly updating it.
Please see below.
+ 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 != _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(_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:
Re: [RFC PATCH 1/2] xen/unpopulated-alloc: Introduce helpers for DMA allocations
On Tue, 17 May 2022, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko
>
> 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
> ---
> 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
> +#include
> #include
> #include
> #include
> @@ -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 != _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(_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
