Re: [PATCH] xen/x86: allow Dom0 PVH to call XENMEM_exchange

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.04.2025 22:19, Stefano Stabellini wrote:
> From: Xenia Ragiadakou <Xenia.Ragiadakou@xxxxxxx>
> 
> Dom0 PVH might need XENMEM_exchange when passing contiguous memory
> addresses to firmware or co-processors not behind an IOMMU.

I definitely don't understand the firmware part: It's subject to the
same transparent P2M translations as the rest of the VM; it's just
another piece of software running there.

"Co-processors not behind an IOMMU" is also interesting; a more
concrete scenario might be nice, yet I realize you may be limited in
what you're allowed to say.

> XENMEM_exchange was blocked for HVM/PVH DomUs, and accidentally it
> impacted Dom0 PVH as well.

This wasn't accidental at all, I don't think.

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4401,7 +4401,7 @@ int steal_page(
>      const struct domain *owner;
>      int rc;
>  
> -    if ( paging_mode_external(d) )
> +    if ( paging_mode_external(d) && !is_hardware_domain(d) )
>          return -EOPNOTSUPP;
>  
>      /* Grab a reference to make sure the page doesn't change under our feet 
> */

Is this (in particular the code following below here) a safe thing to do
when we don't properly refcount page references from the P2M, yet? It's
Dom0, yes, but even there I might see potential security implications (as
top violating privacy of a guest).

Furthermore cleanup_page_mappings() (called later in the function) has a
PV-only aspect which would apparently need widening to PVH Dom0 then,
too.

> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -794,7 +794,7 @@ static long 
> memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
>              rc = guest_physmap_add_page(d, _gfn(gpfn), mfn,
>                                          exch.out.extent_order) ?: rc;
>  
> -            if ( !paging_mode_translate(d) &&
> +            if ( (!paging_mode_translate(d) || is_hardware_domain(d)) &&
>                   __copy_mfn_to_guest_offset(exch.out.extent_start,
>                                              (i << out_chunk_order) + j,
>                                              mfn) )

Wait, no: A PVH domain (Dom0 or not) can't very well make use of MFNs, can
it?

Jan