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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.10.2026 10:12, Juergen Gross wrote:
> On 16.02.26 09:55, Jan Beulich wrote:
>> 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@xxxxxxxx>
>> ---
>> I'm likely screwing things up, as I can't understand how this is intended
>> to work: args->dev_bus_addr shouldn't really be in pseudo-physical address
>> space, or else the address isn't suitable for handing to a device in order
>> to DMA to/from it. Hence using __phys_to_pfn() on it to then hand the
>> result to pfn_to_page() feels bogus. Was all of this perhaps only ever
>> intended for (tested with) translated domains? With the uses of
>> xenmem_reservation_va_mapping_*() only being masquerade?
> 
> I finally forced myself to look into this maze.

Thanks much.

> gnttab_dma_alloc_pages() will only be called via
> gntdev_ioctl_dmabuf_exp_from_refs(), which has:
> 
>       if (xen_pv_domain()) {
>               pr_debug("Cannot provide dma-buf in a PV domain\n");
>               return -EINVAL;
>       }
> 
> at its top.

Well, gnttab_dma_alloc_pages() is an exported function, so it can in
principle be called from anywhere, including out-of-tree modules.

> I'll setup a patch making this clear.
> 
> Are you fine with me taking your patch and cleaning it up regarding the
> use of xenmem_reservation_va_mapping_update()?

Yes, sure.

Jan