[Xen-devel] [PATCH 3/3] memory: restrict XENMEM_remove_from_physmap to translated guests

["Jan Beulich" <JBeulich@xxxxxxxx>]

The commit re-introducing it (14eb3b41d0 ["xen: reinstate previously
unused XENMEM_remove_from_physmap hypercall"]) as well as the one having
originally introduced it (d818f3cb7c ["hvm: Use main memory for video
memory"]) and the one then purging it again (78c3097e4f ["Remove unused
XENMEM_remove_from_physmap"]) make clear that this operation is intended
for use on HVM (i.e. translated) guests only. Restrict it at least as
much, because for PV guests documentation (in the public header) does
not even match the implementation: It talks about GPFN as input, but
get_page_from_gfn() assumes a GMFN in the non-translated case (and hands
back the value passed in).

Also lift the check in XENMEM_add_to_physmap{,_batch} handling up
directly into top level hypercall handling, and clarify things in the
public header accordingly.

Take the liberty and also replace a pointless use of "current" with a
more efficient use of an existing local variable (or function parameter
to be precise).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
TBD: It could be further restricted, disallowing its use by a HVM guest
     on itself.
TBD: Is using P2M_ALLOC here really appropriate? It means e.g.
     pointlessly populating a PoD slot just to unpopulate it again right
     away, with the page then free floating, i.e. no longer available
     for use to replace another PoD slot, and (afaict) no longer
     accessible by the guest in any way.
TBD: Is using guest_physmap_remove_page() here really appropriate? It
     means that e.g. MMIO pages wouldn't be removed. Going through
     guest_remove_page() (while skipping the de-allocation step) would
     seem more appropriate to me, which would address the P2M_ALLOC
     aspect above as well.
     
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4470,9 +4470,6 @@ int xenmem_add_to_physmap_one(
     mfn_t mfn = INVALID_MFN;
     p2m_type_t p2mt;
 
-    if ( !paging_mode_translate(d) )
-        return -EACCES;
-
     switch ( space )
     {
         case XENMAPSPACE_shared_info:
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -815,6 +815,8 @@ int xenmem_add_to_physmap(struct domain
     long rc = 0;
     union xen_add_to_physmap_batch_extra extra;
 
+    ASSERT(paging_mode_translate(d));
+
     if ( xatp->space != XENMAPSPACE_gmfn_foreign )
         extra.res0 = 0;
     else
@@ -997,12 +999,15 @@ static int get_reserved_device_memory(xe
 
 static long xatp_permission_check(struct domain *d, unsigned int space)
 {
+    if ( !paging_mode_translate(d) )
+        return -EACCES;
+
     /*
      * XENMAPSPACE_dev_mmio mapping is only supported for hardware Domain
      * to map this kind of space to itself.
      */
     if ( (space == XENMAPSPACE_dev_mmio) &&
-         (!is_hardware_domain(current->domain) || (d != current->domain)) )
+         (!is_hardware_domain(d) || (d != current->domain)) )
         return -EACCES;
 
     return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
@@ -1386,7 +1391,9 @@ long do_memory_op(unsigned long cmd, XEN
         if ( d == NULL )
             return -ESRCH;
 
-        rc = xsm_remove_from_physmap(XSM_TARGET, curr_d, d);
+        rc = paging_mode_translate(d)
+             ? xsm_remove_from_physmap(XSM_TARGET, curr_d, d)
+             : -EACCES;
         if ( rc )
         {
             rcu_unlock_domain(d);
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -231,7 +231,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_machphys_map
 
 /*
  * Sets the GPFN at which a particular page appears in the specified guest's
- * pseudophysical address space.
+ * physical address space (translated guests only).
  * arg == addr of xen_add_to_physmap_t.
  */
 #define XENMEM_add_to_physmap      7
@@ -298,7 +298,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_add_to_physm
 
 /*
  * Unmaps the page appearing at a particular GPFN from the specified guest's
- * pseudophysical address space.
+ * physical address space (translated guests only).
  * arg == addr of xen_remove_from_physmap_t.
  */
 #define XENMEM_remove_from_physmap      15




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel