[Xen-devel] [PATCH v3 4/5] x86/HVM: fix pinned cache attribute handling

["Jan Beulich" <JBeulich@xxxxxxxx>]

- make sure UC- is only used for PAT purposes (MTRRs and hence EPT
  don't have this type)
- add order input to "get", and properly handle conflict case (forcing
  an EPT page split)
- properly detect (and refuse) overlaps during "set"
- properly use RCU constructs
- support deleting ranges through a special type input to "set"
- set ignore-PAT flag in epte_get_entry_emt() when "get" succeeds
- set "get" output to ~0 (invalid) rather than 0 (UC) on error (the
  caller shouldn't be looking at it anyway)
- move struct hvm_mem_pinned_cacheattr_range from header to C file
  (used only there)

Note that the code (before and after this change) implies the GFN
ranges passed to the hypercall to be inclusive, which is in contrast
to the sole current user in qemu (all variants). It is not clear to me
at which layer (qemu, libxc, hypervisor) this would best be fixed.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Tim Deegan <tim@xxxxxxx>

--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -551,6 +551,15 @@ bool_t mtrr_pat_not_equal(struct vcpu *v
     return 0;
 }
 
+struct hvm_mem_pinned_cacheattr_range {
+    struct list_head list;
+    uint64_t start, end;
+    uint32_t type;
+    struct rcu_head rcu;
+};
+
+static DEFINE_RCU_READ_LOCK(pinned_cacheattr_rcu_lock);
+
 void hvm_init_cacheattr_region_list(
     struct domain *d)
 {
@@ -573,30 +582,47 @@ void hvm_destroy_cacheattr_region_list(
     }
 }
 
-int32_t hvm_get_mem_pinned_cacheattr(
+int hvm_get_mem_pinned_cacheattr(
     struct domain *d,
     uint64_t guest_fn,
+    unsigned int order,
     uint32_t *type)
 {
     struct hvm_mem_pinned_cacheattr_range *range;
+    int rc = 0;
 
-    *type = 0;
+    *type = ~0;
 
     if ( !is_hvm_domain(d) )
         return 0;
 
+    rcu_read_lock(&pinned_cacheattr_rcu_lock);
     list_for_each_entry_rcu ( range,
                               &d->arch.hvm_domain.pinned_cacheattr_ranges,
                               list )
     {
-        if ( (guest_fn >= range->start) && (guest_fn <= range->end) )
+        if ( (guest_fn >= range->start) &&
+             (guest_fn + (1UL << order) - 1 <= range->end) )
         {
             *type = range->type;
-            return 1;
+            rc = 1;
+            break;
+        }
+        if ( (guest_fn <= range->end) &&
+             (range->start <= guest_fn + (1UL << order) - 1) )
+        {
+            rc = -1;
+            break;
         }
     }
+    rcu_read_unlock(&pinned_cacheattr_rcu_lock);
 
-    return 0;
+    return rc;
+}
+
+static void free_pinned_cacheattr_entry(struct rcu_head *rcu)
+{
+    xfree(container_of(rcu, struct hvm_mem_pinned_cacheattr_range, rcu));
 }
 
 int32_t hvm_set_mem_pinned_cacheattr(
@@ -606,6 +632,28 @@ int32_t hvm_set_mem_pinned_cacheattr(
     uint32_t  type)
 {
     struct hvm_mem_pinned_cacheattr_range *range;
+    int rc = 1;
+
+    if ( !is_hvm_domain(d) || gfn_end < gfn_start )
+        return 0;
+
+    if ( type == XEN_DOMCTL_DELETE_MEM_CACHEATTR )
+    {
+        /* Remove the requested range. */
+        rcu_read_lock(&pinned_cacheattr_rcu_lock);
+        list_for_each_entry_rcu ( range,
+                                  &d->arch.hvm_domain.pinned_cacheattr_ranges,
+                                  list )
+            if ( range->start == gfn_start && range->end == gfn_end )
+            {
+                rcu_read_unlock(&pinned_cacheattr_rcu_lock);
+                list_del_rcu(&range->list);
+                call_rcu(&range->rcu, free_pinned_cacheattr_entry);
+                return 0;
+            }
+        rcu_read_unlock(&pinned_cacheattr_rcu_lock);
+        return -ENOENT;
+    }
 
     if ( !((type == PAT_TYPE_UNCACHABLE) ||
            (type == PAT_TYPE_WRCOMB) ||
@@ -616,6 +664,27 @@ int32_t hvm_set_mem_pinned_cacheattr(
          !is_hvm_domain(d) )
         return -EINVAL;
 
+    rcu_read_lock(&pinned_cacheattr_rcu_lock);
+    list_for_each_entry_rcu ( range,
+                              &d->arch.hvm_domain.pinned_cacheattr_ranges,
+                              list )
+    {
+        if ( range->start == gfn_start && range->end == gfn_end )
+        {
+            range->type = type;
+            rc = 0;
+            break;
+        }
+        if ( range->start <= gfn_end && gfn_start <= range->end )
+        {
+            rc = -EBUSY;
+            break;
+        }
+    }
+    rcu_read_unlock(&pinned_cacheattr_rcu_lock);
+    if ( rc <= 0 )
+        return rc;
+
     range = xzalloc(struct hvm_mem_pinned_cacheattr_range);
     if ( range == NULL )
         return -ENOMEM;
@@ -732,8 +801,14 @@ int epte_get_entry_emt(struct domain *d,
     if ( !mfn_valid(mfn_x(mfn)) )
         return MTRR_TYPE_UNCACHABLE;
 
-    if ( hvm_get_mem_pinned_cacheattr(d, gfn, &type) )
-        return type;
+    switch ( hvm_get_mem_pinned_cacheattr(d, gfn, order, &type) )
+    {
+    case 1:
+        *ipat = 1;
+        return type != PAT_TYPE_UC_MINUS ? type : PAT_TYPE_UNCACHABLE;
+    case -1:
+        return -1;
+    }
 
     if ( !iommu_enabled ||
          (rangeset_is_empty(d->iomem_caps) &&
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -601,7 +601,7 @@ _sh_propagate(struct vcpu *v, 
          * 3) if disables snoop control, compute the PAT index with
          *    gMTRR and gPAT.
          */
-        if ( hvm_get_mem_pinned_cacheattr(d, gfn_x(target_gfn), &type) )
+        if ( hvm_get_mem_pinned_cacheattr(d, gfn_x(target_gfn), 0, &type) )
             sflags |= pat_type_2_pte_flags(type);
         else if ( d->arch.hvm_domain.is_in_uc_mode )
             sflags |= pat_type_2_pte_flags(PAT_TYPE_UNCACHABLE);
--- a/xen/include/asm-x86/hvm/cacheattr.h
+++ b/xen/include/asm-x86/hvm/cacheattr.h
@@ -1,12 +1,6 @@
 #ifndef __HVM_CACHEATTR_H__
 #define __HVM_CACHEATTR_H__
 
-struct hvm_mem_pinned_cacheattr_range {
-    struct list_head list;
-    uint64_t start, end;
-    uint32_t type;
-};
-
 void hvm_init_cacheattr_region_list(
     struct domain *d);
 void hvm_destroy_cacheattr_region_list(
@@ -15,11 +9,13 @@ void hvm_destroy_cacheattr_region_list(
 /*
  * To see guest_fn is in the pinned range or not,
  * if yes, return 1, and set type to value in this range
- * if no,  return 0, and set type to 0
+ * if no,  return 0, setting type to ~0
+ * if ambiguous, return -1, setting type to ~0 (possible only for order > 0)
  */
-int32_t hvm_get_mem_pinned_cacheattr(
+int hvm_get_mem_pinned_cacheattr(
     struct domain *d,
     uint64_t guest_fn,
+    unsigned int order,
     uint32_t *type);
 
 
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -555,6 +555,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_iopor
 #define XEN_DOMCTL_MEM_CACHEATTR_WP  5
 #define XEN_DOMCTL_MEM_CACHEATTR_WB  6
 #define XEN_DOMCTL_MEM_CACHEATTR_UCM 7
+#define XEN_DOMCTL_DELETE_MEM_CACHEATTR (~(uint32_t)0)
 struct xen_domctl_pin_mem_cacheattr {
     uint64_aligned_t start, end;
     uint32_t type; /* XEN_DOMCTL_MEM_CACHEATTR_* */

Attachment: x86-HVM-pinned-cache-attr.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel