[Xen-devel] [PATCH-for-4.13 v2] x86/mm: don't needlessly veto migration

[Paul Durrant <paul.durrant@xxxxxxxxxx>]

Now that xl.cfg has an option to explicitly enable IOMMU mappings for a
domain, migration may be needlessly vetoed due to the check of
is_iommu_enabled() in paging_log_dirty_enable().
There is actually no need to prevent logdirty from being enabled unless
devices are assigned to a domain and that domain is sharing HAP mappings
with the IOMMU (in which case disabling write permissions in the P2M may
cause DMA faults). It is quite possible that some assigned devices may
provide information about which pages may have been dirtied by DMA via
an API exported by their managing emulator. Thus Xen's logdirty map is only
one source of information that may be available to the toolstack when
performing a migration and hence it is the toolstack that is best placed
to decide under what circumstances it can be performed, not the hypervisor.

This patch therefore reverts commit 37201c62 "make logdirty and iommu
mutually exclusive" and replaces it with checks to ensure that, if
iommu_use_hap_pt() is true, that logdirty and device assignment are mutually
exclusive.

NOTE: While in the neighbourhood, the bool_t parameter type in
      paging_log_dirty_enable() is replaced with a bool and the format
      of the comment in assign_device() is fixed.

Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
Release-acked-by: Juergen Gross <jgross@xxxxxxxx>
---
Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Wei Liu <wl@xxxxxxx>
Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
Cc: Juergen Gross <jgross@xxxxxxxx>

v2:
 - expand commit comment
---
 xen/arch/x86/mm/hap/hap.c     |  2 +-
 xen/arch/x86/mm/paging.c      |  8 ++++----
 xen/drivers/passthrough/pci.c | 10 +++++++---
 xen/include/asm-x86/paging.h  |  2 +-
 4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 412a442b6a..3d93f3451c 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -71,7 +71,7 @@ int hap_track_dirty_vram(struct domain *d,
 
         if ( !paging_mode_log_dirty(d) )
         {
-            rc = paging_log_dirty_enable(d, 0);
+            rc = paging_log_dirty_enable(d, false);
             if ( rc )
                 goto out;
         }
diff --git a/xen/arch/x86/mm/paging.c b/xen/arch/x86/mm/paging.c
index d9a52c4db4..240f6f93fb 100644
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -209,15 +209,15 @@ static int paging_free_log_dirty_bitmap(struct domain *d, 
int rc)
     return rc;
 }
 
-int paging_log_dirty_enable(struct domain *d, bool_t log_global)
+int paging_log_dirty_enable(struct domain *d, bool log_global)
 {
     int ret;
 
-    if ( is_iommu_enabled(d) && log_global )
+    if ( has_arch_pdevs(d) && iommu_use_hap_pt(d) && log_global )
     {
         /*
          * Refuse to turn on global log-dirty mode
-         * if the domain is using the IOMMU.
+         * if the domain is sharing the P2M with the IOMMU.
          */
         return -EINVAL;
     }
@@ -727,7 +727,7 @@ int paging_domctl(struct domain *d, struct 
xen_domctl_shadow_op *sc,
             break;
         /* Else fall through... */
     case XEN_DOMCTL_SHADOW_OP_ENABLE_LOGDIRTY:
-        return paging_log_dirty_enable(d, 1);
+        return paging_log_dirty_enable(d, true);
 
     case XEN_DOMCTL_SHADOW_OP_OFF:
         if ( (rc = paging_log_dirty_disable(d, resuming)) != 0 )
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 7deef2f12b..9614dca8c1 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1486,11 +1486,15 @@ static int assign_device(struct domain *d, u16 seg, u8 
bus, u8 devfn, u32 flag)
     if ( !is_iommu_enabled(d) )
         return 0;
 
-    /* Prevent device assign if mem paging or mem sharing have been 
-     * enabled for this domain */
+    /*
+     * Prevent device assign if mem paging or mem sharing have been
+     * enabled for this domain, or logdirty is enabled and the P2M is
+     * shared with the IOMMU.
+     */
     if ( unlikely(d->arch.hvm.mem_sharing_enabled ||
                   vm_event_check_ring(d->vm_event_paging) ||
-                  p2m_get_hostp2m(d)->global_logdirty) )
+                  (p2m_get_hostp2m(d)->global_logdirty &&
+                   iommu_use_hap_pt(d))) )
         return -EXDEV;
 
     if ( !pcidevs_trylock() )
diff --git a/xen/include/asm-x86/paging.h b/xen/include/asm-x86/paging.h
index ab7887f23c..8c2027c791 100644
--- a/xen/include/asm-x86/paging.h
+++ b/xen/include/asm-x86/paging.h
@@ -157,7 +157,7 @@ void paging_log_dirty_range(struct domain *d,
                             uint8_t *dirty_bitmap);
 
 /* enable log dirty */
-int paging_log_dirty_enable(struct domain *d, bool_t log_global);
+int paging_log_dirty_enable(struct domain *d, bool log_global);
 
 /* log dirty initialization */
 void paging_log_dirty_init(struct domain *d, const struct log_dirty_ops *ops);
-- 
2.20.1.2.gb21ebb671


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