x86: tighten checks in XEN_DOMCTL_memory_mapping handler

Properly checking the MFN implies knowing the physical address width
supported by the platform, so to obtain this consistently the
respective code gets moved out of the MTRR subdir.

Btw., the model specific workaround in that code is likely unnecessary
- I believe those CPU models don't support 64-bit mode. But I wasn't
able to formally verify this, so I preferred to retain that code for
now.

But domctl code here also was lacking other error checks (as was,
looking at it again from that angle) the XEN_DOMCTL_ioport_mapping one.
Besides adding the missing checks, printing is also added for the case
where revoking access permissions didn't work (as that may have
implications for the host operator, e.g. wanting to not pass through
affected devices to another guest until the one previously using them
did actually die).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 3d0a671edb0cc843d3deafebea3ae6d693d799ab..32f486a1038455c1f2fc438395758910d2dfa1d2 100644 (file)
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -36,6 +36,8 @@ integer_param("cpuid_mask_ext_edx", opt_cpuid_mask_ext_edx);
 
 struct cpu_dev * cpu_devs[X86_VENDOR_NUM] = {};
 
+unsigned int paddr_bits __read_mostly = 36;
+
 /*
  * Default host IA32_CR_PAT value to cover all memory types.
  * BIOS usually sets it to 0x07040600070406.
@@ -265,6 +267,8 @@ static void __cpuinit generic_identify(struct cpuinfo_x86 *c)
                }
                if ( xlvl >= 0x80000004 )
                        get_model_name(c); /* Default name */
+               if ( xlvl >= 0x80000008 )
+                       paddr_bits = cpuid_eax(0x80000008) & 0xff;
        }
 
        /* Intel-defined flags: level 0x00000007 */

diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index f26936ae378c9c97a698e4d2914a3244be5a7589..0d5f25f0637a43673ee0b403ed0885f2973b9d03 100644 (file)
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -144,6 +144,11 @@ void __devinit early_intel_workaround(struct cpuinfo_x86 *c)
                                       c->cpuid_level);
                }
        }
+
+       /* CPUID workaround for Intel 0F33/0F34 CPU */
+       if (boot_cpu_data.x86 == 0xF && boot_cpu_data.x86_model == 3 &&
+           (boot_cpu_data.x86_mask == 3 || boot_cpu_data.x86_mask == 4))
+               paddr_bits = 36;
 }
 
 /*

diff --git a/xen/arch/x86/cpu/mtrr/main.c b/xen/arch/x86/cpu/mtrr/main.c
index dc2d44eb8c377498f107e9e49b6615f5826915a7..fab3a5cf2b5108e88f38b1f5c8fcb69683d3dc18 100644 (file)
--- a/xen/arch/x86/cpu/mtrr/main.c
+++ b/xen/arch/x86/cpu/mtrr/main.c
@@ -559,8 +559,6 @@ struct mtrr_value {
        unsigned long   lsize;
 };
 
-unsigned int paddr_bits __read_mostly = 36;
-
 /**
  * mtrr_bp_init - initialize mtrrs on the boot CPU
  *
@@ -572,31 +570,8 @@ void __init mtrr_bp_init(void)
 {
        if (cpu_has_mtrr) {
                mtrr_if = &generic_mtrr_ops;
-               size_or_mask = 0xff000000;      /* 36 bits */
-               size_and_mask = 0x00f00000;
-
-               /* This is an AMD specific MSR, but we assume(hope?) that
-                  Intel will implement it to when they extend the address
-                  bus of the Xeon. */
-               if (cpuid_eax(0x80000000) >= 0x80000008) {
-                       paddr_bits = cpuid_eax(0x80000008) & 0xff;
-                       /* CPUID workaround for Intel 0F33/0F34 CPU */
-                       if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
-                           boot_cpu_data.x86 == 0xF &&
-                           boot_cpu_data.x86_model == 0x3 &&
-                           (boot_cpu_data.x86_mask == 0x3 ||
-                            boot_cpu_data.x86_mask == 0x4))
-                               paddr_bits = 36;
-
-                       size_or_mask = ~((1ULL << (paddr_bits - PAGE_SHIFT)) - 1);
-                       size_and_mask = ~size_or_mask & 0xfffff00000ULL;
-               } else if (boot_cpu_data.x86_vendor == X86_VENDOR_CENTAUR &&
-                          boot_cpu_data.x86 == 6) {
-                       /* VIA C* family have Intel style MTRRs, but
-                          don't support PAE */
-                       size_or_mask = 0xfff00000;      /* 32 bits */
-                       size_and_mask = 0;
-               }
+               size_or_mask = ~((1ULL << (paddr_bits - PAGE_SHIFT)) - 1);
+               size_and_mask = ~size_or_mask & 0xfffff00000ULL;
        }
 
        if (mtrr_if) {

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index c7f3965b702c46daf4bccaab02212059bb923a11..b407a0470b27fceb6f08fc7c8400f475a1a44ce3 100644 (file)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -825,10 +825,12 @@ long arch_do_domctl(
         unsigned long mfn = domctl->u.memory_mapping.first_mfn;
         unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
         int add = domctl->u.memory_mapping.add_mapping;
-        int i;
+        unsigned long i;
 
         ret = -EINVAL;
-        if ( (mfn + nr_mfns - 1) < mfn ) /* wrap? */
+        if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
+             ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
+             (gfn + nr_mfns - 1) < gfn ) /* wrap? */
             break;
 
         ret = -EPERM;
@@ -853,8 +855,25 @@ long arch_do_domctl(
                    d->domain_id, gfn, mfn, nr_mfns);
 
             ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
-            for ( i = 0; i < nr_mfns; i++ )
-                set_mmio_p2m_entry(d, gfn+i, _mfn(mfn+i));
+            if ( !ret && paging_mode_translate(d) )
+            {
+                for ( i = 0; !ret && i < nr_mfns; i++ )
+                    if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
+                        ret = -EIO;
+                if ( ret )
+                {
+                    printk(XENLOG_G_WARNING
+                           "memory_map:fail: dom%d gfn=%lx mfn=%lx\n",
+                           d->domain_id, gfn + i, mfn + i);
+                    while ( i-- )
+                        clear_mmio_p2m_entry(d, gfn + i);
+                    if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
+                         IS_PRIV(current->domain) )
+                        printk(XENLOG_ERR
+                               "memory_map: failed to deny dom%d access to [%lx,%lx]\n",
+                               d->domain_id, mfn, mfn + nr_mfns - 1);
+                }
+            }
         }
         else
         {
@@ -862,9 +881,17 @@ long arch_do_domctl(
                    "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
                    d->domain_id, gfn, mfn, nr_mfns);
 
-            for ( i = 0; i < nr_mfns; i++ )
-                clear_mmio_p2m_entry(d, gfn+i);
+            if ( paging_mode_translate(d) )
+                for ( i = 0; i < nr_mfns; i++ )
+                    add |= !clear_mmio_p2m_entry(d, gfn + i);
             ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
+            if ( !ret && add )
+                ret = -EIO;
+            if ( ret && IS_PRIV(current->domain) )
+                printk(XENLOG_ERR
+                       "memory_map: error %ld %s dom%d access to [%lx,%lx]\n",
+                       ret, add ? "removing" : "denying", d->domain_id,
+                       mfn, mfn + nr_mfns - 1);
         }
 
         rcu_unlock_domain(d);
@@ -926,12 +953,23 @@ long arch_do_domctl(
             if ( !found )
             {
                 g2m_ioport = xmalloc(struct g2m_ioport);
+                if ( !g2m_ioport )
+                    ret = -ENOMEM;
+            }
+            if ( !found && !ret )
+            {
                 g2m_ioport->gport = fgp;
                 g2m_ioport->mport = fmp;
                 g2m_ioport->np = np;
                 list_add_tail(&g2m_ioport->list, &hd->g2m_ioport_list);
             }
-            ret = ioports_permit_access(d, fmp, fmp + np - 1);
+            if ( !ret )
+                ret = ioports_permit_access(d, fmp, fmp + np - 1);
+            if ( ret && !found && g2m_ioport )
+            {
+                list_del(&g2m_ioport->list);
+                xfree(g2m_ioport);
+            }
         }
         else
         {
@@ -946,6 +984,10 @@ long arch_do_domctl(
                     break;
                 }
             ret = ioports_deny_access(d, fmp, fmp + np - 1);
+            if ( ret && IS_PRIV(current->domain) )
+                printk(XENLOG_ERR
+                       "ioport_map: error %ld denying dom%d access to [%x,%x]\n",
+                       ret, d->domain_id, fmp, fmp + np - 1);
         }
         rcu_unlock_domain(d);
     }