[PATCH v2 0/2] Xen/gnttab: improve error handling in gnttab_dma_{alloc,free}_pages()

[PATCH v2 0/2] Xen/gnttab: improve error handling in gnttab_dma_{alloc,free}_pages()

[Juergen Gross]

This small series is V2 of a patch Jan sent out early this year.
I just added patch 1 due to some remarks Jan made when sending out
his patch, and then removed some now stale parts of his patch.

Jan Beulich (1):
  Xen/gnttab: improve error handling in gnttab_dma_{alloc,free}_pages()

Juergen Gross (1):
  xen: don't allow gnttab_dma_[alloc|free]_pages() in PV guest

 drivers/xen/grant-table.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

-- 
2.56.0

[PATCH v2 1/2] xen: don't allow gnttab_dma_[alloc|free]_pages() in PV guest

[Juergen Gross]

Today gnttab_dma_alloc_pages() or gnttab_dma_free_pages are never
called when running as a PV guest, but this holds for in-tree callers
only.

Bail out early when being called in PV mode in order to ensure no out
of tree module or future use cases are violating this assumption.

This enables to safely remove PV-only code from both functions.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/xen/grant-table.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 604c4eea5930..56a497987942 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -1063,6 +1063,9 @@ int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args)
 	size_t size;
 	int i, ret;
 
+	if (WARN_ON(xen_pv_domain()))
+		return -EINVAL;
+
 	if (args->nr_pages < 0 || args->nr_pages > (INT_MAX >> PAGE_SHIFT))
 		return -ENOMEM;
 
@@ -1090,8 +1093,6 @@ int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args)
 		xenmem_reservation_scrub_page(page);
 	}
 
-	xenmem_reservation_va_mapping_reset(args->nr_pages, args->pages);
-
 	ret = xenmem_reservation_decrease(args->nr_pages, args->frames);
 	if (ret != args->nr_pages) {
 		pr_debug("Failed to decrease reservation for DMA buffer\n");
@@ -1120,6 +1121,9 @@ int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args)
 	size_t size;
 	int i, ret;
 
+	if (WARN_ON(xen_pv_domain()))
+		return -EINVAL;
+
 	gnttab_pages_clear_private(args->nr_pages, args->pages);
 
 	for (i = 0; i < args->nr_pages; i++)
@@ -1133,9 +1137,6 @@ int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args)
 		ret = 0;
 	}
 
-	xenmem_reservation_va_mapping_update(args->nr_pages, args->pages,
-					     args->frames);
-
 	size = args->nr_pages << PAGE_SHIFT;
 	if (args->coherent)
 		dma_free_coherent(args->dev, size,
-- 
2.56.0

[PATCH v2 2/2] Xen/gnttab: improve error handling in gnttab_dma_{alloc,free}_pages()

[Juergen Gross]

From: Jan Beulich <jbeulich@suse.com>

Both decrease-reservation and populate-physmap can succeed partially. For
the former this may be pretty unlikely, but clearly the latter can hit
both out-of-memory conditions in the hypervisor or denial because of the
allocation exceeding the domain's allowance (there's no interaction with
the balloon driver here either).

In gnttab_dma_free_pages() we simply can't give back to the system what
hasn't been re-filled with backing memory. For gnttab_dma_alloc_pages()
both parts of the overall region need dealing with differently.

While no present caller of gnttab_dma_free_pages() checks its return
value, also don't use -EFAULT there: It really is an out-of-memory
condition. (In gnttab_dma_alloc_pages() -EFAULT doesn't look quite right
either, but I couldn't think of a clearly better error code there.)

Fixes: 9bdc7304f536 ("xen/grant-table: Allow allocating buffers suitable for DMA")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/xen/grant-table.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 56a497987942..2a520080a570 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -1096,6 +1096,24 @@ int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args)
 	ret = xenmem_reservation_decrease(args->nr_pages, args->frames);
 	if (ret != args->nr_pages) {
 		pr_debug("Failed to decrease reservation for DMA buffer\n");
+		if (ret >= 0) {
+			/* Free the part where decrease didn't work. */
+			size_t done = ret << PAGE_SHIFT;
+
+			if (args->coherent)
+				dma_free_coherent(args->dev, size - done,
+						  args->vaddr + done,
+						  args->dev_bus_addr + done);
+			else
+				dma_free_wc(args->dev, size - done,
+					    args->vaddr + done,
+					    args->dev_bus_addr + done);
+
+			if (!ret) /* Nothing else to do? */
+				return -EFAULT;
+
+			args->nr_pages = ret;
+		}
 		ret = -EFAULT;
 		goto fail;
 	}
@@ -1132,7 +1150,12 @@ int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args)
 	ret = xenmem_reservation_increase(args->nr_pages, args->frames);
 	if (ret != args->nr_pages) {
 		pr_debug("Failed to increase reservation for DMA buffer\n");
-		ret = -EFAULT;
+		/* Can only free what has been re-filled. */
+		if (!ret)
+			return -ENOMEM;
+		if (ret > 0)
+			args->nr_pages = ret;
+		ret = -ENOMEM;
 	} else {
 		ret = 0;
 	}
-- 
2.56.0

Re: [PATCH v2 1/2] xen: don't allow gnttab_dma_[alloc|free]_pages() in PV guest

[Andrew Cooper]

On 10/11/2026 9:56 am, Juergen Gross wrote:
> Today gnttab_dma_alloc_pages() or gnttab_dma_free_pages are never
> called when running as a PV guest, but this holds for in-tree callers
> only.
>
> Bail out early when being called in PV mode in order to ensure no out
> of tree module or future use cases are violating this assumption.
>
> This enables to safely remove PV-only code from both functions.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  drivers/xen/grant-table.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 604c4eea5930..56a497987942 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -1063,6 +1063,9 @@ int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args)
>  	size_t size;
>  	int i, ret;
>  
> +	if (WARN_ON(xen_pv_domain()))
> +		return -EINVAL;

This will get very spammy if hit.  WARN_ON_ONCE() ?

~Andrew

Re: [PATCH v2 1/2] xen: don't allow gnttab_dma_[alloc|free]_pages() in PV guest

[Jürgen Groß]

[-- Attachment #1.1: Type: text/plain, Size: 1260 bytes --]

On 10.11.26 10:59, Andrew Cooper wrote:
> On 10/11/2026 9:56 am, Juergen Gross wrote:
>> Today gnttab_dma_alloc_pages() or gnttab_dma_free_pages are never
>> called when running as a PV guest, but this holds for in-tree callers
>> only.
>>
>> Bail out early when being called in PV mode in order to ensure no out
>> of tree module or future use cases are violating this assumption.
>>
>> This enables to safely remove PV-only code from both functions.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   drivers/xen/grant-table.c | 11 ++++++-----
>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
>> index 604c4eea5930..56a497987942 100644
>> --- a/drivers/xen/grant-table.c
>> +++ b/drivers/xen/grant-table.c
>> @@ -1063,6 +1063,9 @@ int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args)
>>   	size_t size;
>>   	int i, ret;
>>   
>> +	if (WARN_ON(xen_pv_domain()))
>> +		return -EINVAL;
> 
> This will get very spammy if hit.  WARN_ON_ONCE() ?
I'd rather have it spammy than buried somewhere in the logs.

Additionally I expect any illegal user giving up quite fast if no allocation
at all for DMA is working. :-)


Juergen

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 0/2] Xen/gnttab: improve error handling in gnttab_dma_{alloc,free}_pages()

[Jan Beulich]

On 10.11.2026 10:56, Juergen Gross wrote:
> This small series is V2 of a patch Jan sent out early this year.
> I just added patch 1 due to some remarks Jan made when sending out
> his patch, and then removed some now stale parts of his patch.
> 
> Jan Beulich (1):
>   Xen/gnttab: improve error handling in gnttab_dma_{alloc,free}_pages()
> 
> Juergen Gross (1):
>   xen: don't allow gnttab_dma_[alloc|free]_pages() in PV guest
> 
>  drivers/xen/grant-table.c | 36 ++++++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)

Reviewed-by: Jan Beulich <jbeulich@suse.com>

(Whether that's meaningful in the Linux world on patch 1 I don't know.)

Jan