[Xen-changelog] [xen stable-4.6] x86: fix (and simplify) MTRR overlap checking

[patchbot@xxxxxxx]

commit b514aeccb1222229008009361cafa1d5b0d764cb
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Wed Feb 17 16:37:14 2016 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Wed Feb 17 16:37:14 2016 +0100

    x86: fix (and simplify) MTRR overlap checking
    
    Obtaining one individual range per variable range register (via
    get_mtrr_range()) was bogus from the beginning, as these registers may
    cover multiple disjoint ranges. Do away with that, in favor of simply
    comparing masked addresses.
    
    Also, for is_var_mtrr_overlapped()'s result to be correct when called
    from mtrr_wrmsr(), generic_set_mtrr() must update saved state first.
    
    As minor cleanup changes, constify is_var_mtrr_overlapped()'s parameter
    and make mtrr_wrmsr() static.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    master commit: 3272230848f36eb5bbb660216898a90048a81d9f
    master date: 2016-01-21 16:11:04 +0100
---
 xen/arch/x86/cpu/mtrr/generic.c |  4 +--
 xen/arch/x86/cpu/mtrr/mtrr.h    |  1 -
 xen/arch/x86/hvm/mtrr.c         | 61 ++++++++---------------------------------
 xen/include/asm-x86/mtrr.h      |  2 +-
 4 files changed, 15 insertions(+), 53 deletions(-)

diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index 935f0a0..ea0efe2 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -220,7 +220,7 @@ void __init mtrr_state_warn(void)
 /* Doesn't attempt to pass an error out to MTRR users
    because it's quite complicated in some cases and probably not
    worth it because the best error handling is to ignore it. */
-void mtrr_wrmsr(unsigned int msr, uint64_t msr_content)
+static void mtrr_wrmsr(unsigned int msr, uint64_t msr_content)
 {
        if (wrmsr_safe(msr, msr_content) < 0)
                printk(KERN_ERR
@@ -495,8 +495,8 @@ static void generic_set_mtrr(unsigned int reg, unsigned 
long base,
        if (size == 0) {
                /* The invalid bit is kept in the mask, so we simply clear the
                   relevant mask register to disable a range. */
+               memset(vr, 0, sizeof(*vr));
                mtrr_wrmsr(MSR_IA32_MTRR_PHYSMASK(reg), 0);
-               memset(vr, 0, sizeof(struct mtrr_var_range));
        } else {
                uint32_t base_lo, base_hi, mask_lo, mask_hi;
 
diff --git a/xen/arch/x86/cpu/mtrr/mtrr.h b/xen/arch/x86/cpu/mtrr/mtrr.h
index a9d6dcd..b41eb58 100644
--- a/xen/arch/x86/cpu/mtrr/mtrr.h
+++ b/xen/arch/x86/cpu/mtrr/mtrr.h
@@ -63,7 +63,6 @@ extern const struct mtrr_ops *mtrr_if;
 extern unsigned int num_var_ranges;
 
 void mtrr_state_warn(void);
-void mtrr_wrmsr(unsigned int msr, uint64_t msr_content);
 
 extern int amd_init_mtrr(void);
 extern int cyrix_init_mtrr(void);
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index 9268dfb..4188ae7 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -26,8 +26,6 @@
 #include <asm/hvm/support.h>
 #include <asm/hvm/cacheattr.h>
 
-static uint32_t size_or_mask;
-
 /* Get page attribute fields (PAn) from PAT MSR. */
 #define pat_cr_2_paf(pat_cr,n)  ((((uint64_t)pat_cr) >> ((n)<<3)) & 0xff)
 
@@ -77,61 +75,28 @@ static uint8_t __read_mostly 
mtrr_epat_tbl[MTRR_NUM_TYPES][MEMORY_NUM_TYPES] =
 static uint8_t __read_mostly pat_entry_tbl[PAT_TYPE_NUMS] =
     { [0 ... PAT_TYPE_NUMS-1] = INVALID_MEM_TYPE };
 
-static void get_mtrr_range(uint64_t base_msr, uint64_t mask_msr,
-                           uint64_t *base, uint64_t *end)
-{
-    uint32_t mask_lo = (uint32_t)mask_msr;
-    uint32_t mask_hi = (uint32_t)(mask_msr >> 32);
-    uint32_t base_lo = (uint32_t)base_msr;
-    uint32_t base_hi = (uint32_t)(base_msr >> 32);
-    uint32_t size;
-
-    if ( !(mask_lo & MTRR_PHYSMASK_VALID) )
-    {
-        /* Invalid (i.e. free) range */
-        *base = 0;
-        *end = 0;
-        return;
-    }
-
-    /* Work out the shifted address mask. */
-    mask_lo = (size_or_mask | (mask_hi << (32 - PAGE_SHIFT)) |
-               (mask_lo >> PAGE_SHIFT));
-
-    /* This works correctly if size is a power of two (a contiguous range). */
-    size = -mask_lo;
-    *base = base_hi << (32 - PAGE_SHIFT) | base_lo >> PAGE_SHIFT;
-    *end = *base + size - 1;
-}
-
-bool_t is_var_mtrr_overlapped(struct mtrr_state *m)
+bool_t is_var_mtrr_overlapped(const struct mtrr_state *m)
 {
-    int32_t seg, i;
-    uint64_t phys_base, phys_mask, phys_base_pre, phys_mask_pre;
-    uint64_t base_pre, end_pre, base, end;
-    uint8_t num_var_ranges = (uint8_t)m->mtrr_cap;
+    unsigned int seg, i;
+    unsigned int num_var_ranges = (uint8_t)m->mtrr_cap;
 
     for ( i = 0; i < num_var_ranges; i++ )
     {
-        phys_base_pre = ((uint64_t*)m->var_ranges)[i*2];
-        phys_mask_pre = ((uint64_t*)m->var_ranges)[i*2 + 1];
+        uint64_t base1 = m->var_ranges[i].base >> PAGE_SHIFT;
+        uint64_t mask1 = m->var_ranges[i].mask >> PAGE_SHIFT;
 
-        get_mtrr_range(phys_base_pre, phys_mask_pre,
-                        &base_pre, &end_pre);
+        if ( !(m->var_ranges[i].mask & MTRR_PHYSMASK_VALID) )
+            continue;
 
         for ( seg = i + 1; seg < num_var_ranges; seg ++ )
         {
-            phys_base = ((uint64_t*)m->var_ranges)[seg*2];
-            phys_mask = ((uint64_t*)m->var_ranges)[seg*2 + 1];
+            uint64_t base2 = m->var_ranges[seg].base >> PAGE_SHIFT;
+            uint64_t mask2 = m->var_ranges[seg].mask >> PAGE_SHIFT;
 
-            get_mtrr_range(phys_base, phys_mask,
-                            &base, &end);
+            if ( !(m->var_ranges[seg].mask & MTRR_PHYSMASK_VALID) )
+                continue;
 
-            if ( ((base_pre != end_pre) && (base != end))
-                 || ((base >= base_pre) && (base <= end_pre))
-                 || ((end >= base_pre) && (end <= end_pre))
-                 || ((base_pre >= base) && (base_pre <= end))
-                 || ((end_pre >= base) && (end_pre <= end)) )
+            if ( (base1 & mask1 & mask2) == (base2 & mask2 & mask1) )
             {
                 /* MTRR is overlapped. */
                 return 1;
@@ -168,8 +133,6 @@ static int __init hvm_mtrr_pat_init(void)
         }
     }
 
-    size_or_mask = ~((1 << (paddr_bits - PAGE_SHIFT)) - 1);
-
     return 0;
 }
 __initcall(hvm_mtrr_pat_init);
diff --git a/xen/include/asm-x86/mtrr.h b/xen/include/asm-x86/mtrr.h
index 0569db6..69cf68c 100644
--- a/xen/include/asm-x86/mtrr.h
+++ b/xen/include/asm-x86/mtrr.h
@@ -91,7 +91,7 @@ extern bool_t mtrr_def_type_msr_set(struct domain *, struct 
mtrr_state *,
 extern void memory_type_changed(struct domain *);
 extern bool_t pat_msr_set(uint64_t *pat, uint64_t msr);
 
-bool_t is_var_mtrr_overlapped(struct mtrr_state *m);
+bool_t is_var_mtrr_overlapped(const struct mtrr_state *m);
 bool_t mtrr_pat_not_equal(struct vcpu *vd, struct vcpu *vs);
 
 #endif /* __ASM_X86_MTRR_H__ */
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.6

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog