As documented in comments in struct xen_memory_exchange, the input to the
hypercall is a set of MFNs which are to be removed from the domain, plus a
set of PFNs where the newly allocated MFNs are to appear. Present comment
and parameter naming don't correctly reflect that.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Should "fns_out" maybe be "fns_inout" to further emphasize the dual
purpose?

--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2291,18 +2291,19 @@ static void xen_remap_exchanged_ptes(uns
 }
 
 /*
- * Perform the hypercall to exchange a region of our pfns to point to
- * memory with the required contiguous alignment.  Takes the pfns as
- * input, and populates mfns as output.
+ * Perform the hypercall to exchange a region of our pages to point to memory
+ * with the required contiguous alignment.  Takes as input the mfns to trade
+ * in and the pfns where the new pages are to appear, and populates mfns as
+ * output.
  *
  * Returns a success code indicating whether the hypervisor was able to
  * satisfy the request or not.
  */
 static int xen_exchange_memory(unsigned long extents_in, unsigned int order_in,
-			       unsigned long *pfns_in,
+			       unsigned long *mfns_in,
 			       unsigned long extents_out,
 			       unsigned int order_out,
-			       unsigned long *mfns_out,
+			       unsigned long *fns_out,
 			       unsigned int address_bits)
 {
 	long rc;
@@ -2312,13 +2313,13 @@ static int xen_exchange_memory(unsigned
 		.in = {
 			.nr_extents   = extents_in,
 			.extent_order = order_in,
-			.extent_start = pfns_in,
+			.extent_start = mfns_in,
 			.domid        = DOMID_SELF
 		},
 		.out = {
 			.nr_extents   = extents_out,
 			.extent_order = order_out,
-			.extent_start = mfns_out,
+			.extent_start = fns_out,
 			.address_bits = address_bits,
 			.domid        = DOMID_SELF
 		}

On 01.07.26 11:55, Jan Beulich wrote:
> As documented in comments in struct xen_memory_exchange, the input to the
> hypercall is a set of MFNs which are to be removed from the domain, plus a
> set of PFNs where the newly allocated MFNs are to appear. Present comment
> and parameter naming don't correctly reflect that.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Should "fns_out" maybe be "fns_inout" to further emphasize the dual
> purpose?

I think so, yes. And I'd like to have that reflected in the comment you are
modifying ...

> 
> --- a/arch/x86/xen/mmu_pv.c
> +++ b/arch/x86/xen/mmu_pv.c
> @@ -2291,18 +2291,19 @@ static void xen_remap_exchanged_ptes(uns
>   }
>   
>   /*
> - * Perform the hypercall to exchange a region of our pfns to point to
> - * memory with the required contiguous alignment.  Takes the pfns as
> - * input, and populates mfns as output.
> + * Perform the hypercall to exchange a region of our pages to point to memory
> + * with the required contiguous alignment.  Takes as input the mfns to trade
> + * in and the pfns where the new pages are to appear, and populates mfns as
> + * output.

... like:

   [...] Takes as input the mfns to trade in (mfns_in) and the pfns where the new
   pages are to appear (fns_inout), and populates mfns as output (fns_inout).

With that:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen