[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen-unstable] vmce: Clean up implementation and setup/destroy.



# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1273142349 -3600
# Node ID 26da9bb87405c64c02def8d5f11c66f15847bd02
# Parent  24a00bbe7cc0c1dd1293ac78f98b30399db64343
vmce: Clean up implementation and setup/destroy.

Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
---
 xen/arch/x86/cpu/mcheck/vmce.c |  274 +++++++++++++++++++----------------------
 xen/arch/x86/domain.c          |    2 
 xen/common/domain.c            |    2 
 xen/include/asm-x86/mce.h      |    3 
 4 files changed, 131 insertions(+), 150 deletions(-)

diff -r 24a00bbe7cc0 -r 26da9bb87405 xen/arch/x86/cpu/mcheck/vmce.c
--- a/xen/arch/x86/cpu/mcheck/vmce.c    Thu May 06 11:19:28 2010 +0100
+++ b/xen/arch/x86/cpu/mcheck/vmce.c    Thu May 06 11:39:09 2010 +0100
@@ -18,15 +18,10 @@
 #include "mce.h"
 #include "x86_mca.h"
 
+#define dom_vmce(x)   ((x)->arch.vmca_msrs)
+
 int vmce_init_msr(struct domain *d)
 {
-    if ( dom_vmce(d) )
-    {
-        dprintk(XENLOG_G_WARNING, "Domain %d has inited vMCE\n", d->domain_id);
-        return 0;
-    }
-
-    /* Allocate the vmca_msrs and mci_ctl togother */
     dom_vmce(d) = xmalloc(struct domain_mca_msrs);
     if ( !dom_vmce(d) )
         return -ENOMEM;
@@ -37,72 +32,73 @@ int vmce_init_msr(struct domain *d)
         xfree(dom_vmce(d));
         return -ENOMEM;
     }
-    memset(d->arch.vmca_msrs->mci_ctl, ~0,
-           sizeof(d->arch.vmca_msrs->mci_ctl));
+    memset(dom_vmce(d)->mci_ctl, ~0,
+           sizeof(dom_vmce(d)->mci_ctl));
 
     dom_vmce(d)->mcg_status = 0x0;
     dom_vmce(d)->mcg_cap = g_mcg_cap;
     dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
     dom_vmce(d)->nr_injection = 0;
 
-    INIT_LIST_HEAD(&d->arch.vmca_msrs->impact_header);
-    spin_lock_init(&d->arch.vmca_msrs->lock);
+    INIT_LIST_HEAD(&dom_vmce(d)->impact_header);
+    spin_lock_init(&dom_vmce(d)->lock);
 
     return 0;
 }
 
-/*
- * Caller should make sure msr is bank msr */
+void vmce_destroy_msr(struct domain *d)
+{
+    if ( !dom_vmce(d) )
+        return;
+    xfree(dom_vmce(d)->mci_ctl);
+    xfree(dom_vmce(d));
+    dom_vmce(d) = NULL;
+}
+
 static int bank_mce_rdmsr(struct domain *d, uint32_t msr, uint64_t *val)
 {
     int bank, ret = 1;
-    struct domain_mca_msrs *vmce;
-    struct bank_entry *entry = NULL;
-
-    if (!d)
-        return -EINVAL;
-    vmce = dom_vmce(d);
-    ASSERT(vmce);
+    struct domain_mca_msrs *vmce = dom_vmce(d);
+    struct bank_entry *entry;
 
     bank = (msr - MSR_IA32_MC0_CTL) / 4;
-    if (bank >= nr_mce_banks)
+    if ( bank >= nr_mce_banks )
         return -1;
 
-    switch (msr & (MSR_IA32_MC0_CTL | 3))
+    switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
         *val = vmce->mci_ctl[bank] &
-          (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
+            (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
         mce_printk(MCE_VERBOSE, "MCE: rdmsr MC%u_CTL 0x%"PRIx64"\n",
-          bank, *val);
+                   bank, *val);
         break;
     case MSR_IA32_MC0_STATUS:
         /* Only error bank is read. Non-error banks simply return. */
         if ( !list_empty(&vmce->impact_header) )
         {
             entry = list_entry(vmce->impact_header.next,
-              struct bank_entry, list);
-            if (entry->bank == bank) {
+                               struct bank_entry, list);
+            if ( entry->bank == bank )
+            {
                 *val = entry->mci_status;
                 mce_printk(MCE_VERBOSE,
-                  "MCE: rd MC%u_STATUS in vMCE# context "
-                  "value 0x%"PRIx64"\n", bank, *val);
-            }
-            else
-                entry = NULL;
+                           "MCE: rd MC%u_STATUS in vMCE# context "
+                           "value 0x%"PRIx64"\n", bank, *val);
+            }
         }
         break;
     case MSR_IA32_MC0_ADDR:
         if ( !list_empty(&vmce->impact_header) )
         {
             entry = list_entry(vmce->impact_header.next,
-              struct bank_entry, list);
+                               struct bank_entry, list);
             if ( entry->bank == bank )
             {
                 *val = entry->mci_addr;
                 mce_printk(MCE_VERBOSE,
-                  "MCE: rdmsr MC%u_ADDR in vMCE# context "
-                  "0x%"PRIx64"\n", bank, *val);
+                           "MCE: rdmsr MC%u_ADDR in vMCE# context "
+                           "0x%"PRIx64"\n", bank, *val);
             }
         }
         break;
@@ -110,25 +106,25 @@ static int bank_mce_rdmsr(struct domain 
         if ( !list_empty(&vmce->impact_header) )
         {
             entry = list_entry(vmce->impact_header.next,
-              struct bank_entry, list);
+                               struct bank_entry, list);
             if ( entry->bank == bank )
             {
                 *val = entry->mci_misc;
                 mce_printk(MCE_VERBOSE,
-                  "MCE: rd MC%u_MISC in vMCE# context "
-                  "0x%"PRIx64"\n", bank, *val);
+                           "MCE: rd MC%u_MISC in vMCE# context "
+                           "0x%"PRIx64"\n", bank, *val);
             }
         }
         break;
     default:
         switch ( boot_cpu_data.x86_vendor )
         {
-            case X86_VENDOR_INTEL:
-                ret = intel_mce_rdmsr(msr, val);
-                break;
-            default:
-                ret = 0;
-                break;
+        case X86_VENDOR_INTEL:
+            ret = intel_mce_rdmsr(msr, val);
+            break;
+        default:
+            ret = 0;
+            break;
         }
         break;
     }
@@ -144,19 +140,12 @@ int vmce_rdmsr(uint32_t msr, uint64_t *v
 int vmce_rdmsr(uint32_t msr, uint64_t *val)
 {
     struct domain *d = current->domain;
-    struct domain_mca_msrs *vmce;
+    struct domain_mca_msrs *vmce = dom_vmce(d);
     int ret = 1;
 
     *val = 0;
 
-    vmce = dom_vmce(d);
-    if ( !vmce )
-    {
-        /* XXX more handle here */
-        return 0;
-    }
-
-    spin_lock(&d->arch.vmca_msrs->lock);
+    spin_lock(&dom_vmce(d)->lock);
 
     switch ( msr )
     {
@@ -164,79 +153,71 @@ int vmce_rdmsr(uint32_t msr, uint64_t *v
         *val = vmce->mcg_status;
         if (*val)
             mce_printk(MCE_VERBOSE,
-                "MCE: rdmsr MCG_STATUS 0x%"PRIx64"\n", *val);
+                       "MCE: rdmsr MCG_STATUS 0x%"PRIx64"\n", *val);
         break;
     case MSR_IA32_MCG_CAP:
         *val = vmce->mcg_cap;
         mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CAP 0x%"PRIx64"\n",
-            *val);
+                   *val);
         break;
     case MSR_IA32_MCG_CTL:
         /* Always 0 if no CTL support */
         *val = vmce->mcg_ctl & h_mcg_ctl;
         mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CTL 0x%"PRIx64"\n",
-            *val);
+                   *val);
         break;
     default:
-        if ( mce_bank_msr(msr) )
-            ret = bank_mce_rdmsr(d, msr, val);
-        else
-            ret = 0;
-        break;
-    }
-
-    spin_unlock(&d->arch.vmca_msrs->lock);
+        ret = mce_bank_msr(msr) ? bank_mce_rdmsr(d, msr, val) : 0;
+        break;
+    }
+
+    spin_unlock(&dom_vmce(d)->lock);
     return ret;
 }
 
-int bank_mce_wrmsr(struct domain *d, u32 msr, u64 val)
+static int bank_mce_wrmsr(struct domain *d, u32 msr, u64 val)
 {
     int bank, ret = 1;
-    struct domain_mca_msrs *vmce;
+    struct domain_mca_msrs *vmce = dom_vmce(d);
     struct bank_entry *entry = NULL;
 
-    if (!d)
-        return -EINVAL;
-    vmce = dom_vmce(d);
-    ASSERT(vmce && vmce->mci_ctl);
-
     bank = (msr - MSR_IA32_MC0_CTL) / 4;
-    if (bank >= nr_mce_banks)
+    if ( bank >= nr_mce_banks )
         return -EINVAL;
 
     switch ( msr & (MSR_IA32_MC0_CTL | 3) )
     {
     case MSR_IA32_MC0_CTL:
         vmce->mci_ctl[bank] = val;
-            break;
+        break;
     case MSR_IA32_MC0_STATUS:
-            /* Give the first entry of the list, it corresponds to current
-             * vMCE# injection. When vMCE# is finished processing by the
-             * the guest, this node will be deleted.
-             * Only error bank is written. Non-error banks simply return.
-             */
-            if ( !list_empty(&d->arch.vmca_msrs->impact_header) )
-            {
-                entry = list_entry(d->arch.vmca_msrs->impact_header.next,
-                                   struct bank_entry, list);
-                if ( entry->bank == bank )
-                    entry->mci_status = val;
-                mce_printk(MCE_VERBOSE,
-                         "MCE: wr MC%u_STATUS %"PRIx64" in vMCE#\n",
-                         bank, val);
-            }
-            else
-                mce_printk(MCE_VERBOSE,
-                         "MCE: wr MC%u_STATUS %"PRIx64"\n", bank, val);
-            break;
+        /* Give the first entry of the list, it corresponds to current
+         * vMCE# injection. When vMCE# is finished processing by the
+         * the guest, this node will be deleted.
+         * Only error bank is written. Non-error banks simply return.
+         */
+        if ( !list_empty(&dom_vmce(d)->impact_header) )
+        {
+            entry = list_entry(dom_vmce(d)->impact_header.next,
+                               struct bank_entry, list);
+            if ( entry->bank == bank )
+                entry->mci_status = val;
+            mce_printk(MCE_VERBOSE,
+                       "MCE: wr MC%u_STATUS %"PRIx64" in vMCE#\n",
+                       bank, val);
+        }
+        else
+            mce_printk(MCE_VERBOSE,
+                       "MCE: wr MC%u_STATUS %"PRIx64"\n", bank, val);
+        break;
     case MSR_IA32_MC0_ADDR:
-            mce_printk(MCE_QUIET, "MCE: MC%u_ADDR is read-only\n", bank);
-            ret = -1;
-            break;
+        mce_printk(MCE_QUIET, "MCE: MC%u_ADDR is read-only\n", bank);
+        ret = -1;
+        break;
     case MSR_IA32_MC0_MISC:
-            mce_printk(MCE_QUIET, "MCE: MC%u_MISC is read-only\n", bank);
-            ret = -1;
-            break;
+        mce_printk(MCE_QUIET, "MCE: MC%u_MISC is read-only\n", bank);
+        ret = -1;
+        break;
     default:
         switch ( boot_cpu_data.x86_vendor )
         {
@@ -262,13 +243,12 @@ int vmce_wrmsr(u32 msr, u64 val)
 {
     struct domain *d = current->domain;
     struct bank_entry *entry = NULL;
-    struct domain_mca_msrs *vmce;
+    struct domain_mca_msrs *vmce = dom_vmce(d);
     int ret = 1;
 
     if ( !g_mcg_cap )
         return 0;
 
-    vmce = dom_vmce(d);
     spin_lock(&vmce->lock);
 
     switch ( msr )
@@ -286,20 +266,20 @@ int vmce_wrmsr(u32 msr, u64 val)
             if ( !list_empty(&vmce->impact_header) )
             {
                 entry = list_entry(vmce->impact_header.next,
-                    struct bank_entry, list);
+                                   struct bank_entry, list);
                 if ( entry->mci_status & MCi_STATUS_VAL )
                     mce_printk(MCE_QUIET, "MCE: MCi_STATUS MSR should have "
-                                "been cleared before write MCG_STATUS MSR\n");
+                               "been cleared before write MCG_STATUS MSR\n");
 
                 mce_printk(MCE_QUIET, "MCE: Delete HVM last injection "
-                                "Node, nr_injection %u\n",
-                                vmce->nr_injection);
+                           "Node, nr_injection %u\n",
+                           vmce->nr_injection);
                 list_del(&entry->list);
                 xfree(entry);
             }
             else
                 mce_printk(MCE_QUIET, "MCE: Not found HVM guest"
-                    " last injection Node, something Wrong!\n");
+                           " last injection Node, something Wrong!\n");
         }
         break;
     case MSR_IA32_MCG_CAP:
@@ -307,10 +287,7 @@ int vmce_wrmsr(u32 msr, u64 val)
         ret = -1;
         break;
     default:
-        if ( mce_bank_msr(msr) )
-            ret = bank_mce_wrmsr(d, msr, val);
-        else
-            ret = 0;
+        ret = mce_bank_msr(msr) ? bank_mce_wrmsr(d, msr, val) : 0;
         break;
     }
 
@@ -323,47 +300,46 @@ int inject_vmce(struct domain *d)
     int cpu = smp_processor_id();
     cpumask_t affinity;
 
-    /* PV guest and HVM guest have different vMCE# injection
-     * methods*/
+    /* PV guest and HVM guest have different vMCE# injection methods. */
     if ( !test_and_set_bool(d->vcpu[0]->mce_pending) )
     {
-        if (d->is_hvm)
+        if ( d->is_hvm )
         {
             mce_printk(MCE_VERBOSE, "MCE: inject vMCE to HVM DOM %d\n",
-                        d->domain_id);
+                       d->domain_id);
             vcpu_kick(d->vcpu[0]);
         }
-        /* PV guest including DOM0 */
         else
         {
             mce_printk(MCE_VERBOSE, "MCE: inject vMCE to PV DOM%d\n",
-                        d->domain_id);
-            if (guest_has_trap_callback
-                   (d, 0, TRAP_machine_check))
+                       d->domain_id);
+            if ( guest_has_trap_callback(d, 0, TRAP_machine_check) )
             {
                 d->vcpu[0]->cpu_affinity_tmp =
-                        d->vcpu[0]->cpu_affinity;
+                    d->vcpu[0]->cpu_affinity;
                 cpus_clear(affinity);
                 cpu_set(cpu, affinity);
-                mce_printk(MCE_VERBOSE, "MCE: CPU%d set affinity, old %d\n", 
cpu,
-                            d->vcpu[0]->processor);
+                mce_printk(MCE_VERBOSE, "MCE: CPU%d set affinity, old %d\n",
+                           cpu, d->vcpu[0]->processor);
                 vcpu_set_affinity(d->vcpu[0], &affinity);
                 vcpu_kick(d->vcpu[0]);
             }
             else
             {
-                mce_printk(MCE_VERBOSE, "MCE: Kill PV guest with No MCE 
handler\n");
+                mce_printk(MCE_VERBOSE,
+                           "MCE: Kill PV guest with No MCE handler\n");
                 domain_crash(d);
             }
         }
     }
-    else {
+    else
+    {
         /* new vMCE comes while first one has not been injected yet,
          * in this case, inject fail. [We can't lose this vMCE for
          * the mce node's consistency].
-        */
+         */
         mce_printk(MCE_QUIET, "There's a pending vMCE waiting to be injected "
-                    " to this DOM%d!\n", d->domain_id);
+                   " to this DOM%d!\n", d->domain_id);
         return -1;
     }
     return 0;
@@ -376,14 +352,17 @@ int inject_vmce(struct domain *d)
  * processed by guest, the corresponding node will be deleted.
  * This node list is for GUEST vMCE# MSRS virtualization.
  */
-static struct bank_entry* alloc_bank_entry(void) {
+static struct bank_entry* alloc_bank_entry(void)
+{
     struct bank_entry *entry;
 
     entry = xmalloc(struct bank_entry);
-    if (!entry) {
+    if ( entry == NULL )
+    {
         printk(KERN_ERR "MCE: malloc bank_entry failed\n");
         return NULL;
     }
+
     memset(entry, 0x0, sizeof(entry));
     INIT_LIST_HEAD(&entry->list);
     return entry;
@@ -393,27 +372,28 @@ static struct bank_entry* alloc_bank_ent
  * MSR virtualization data
  * 1) Log down how many nr_injections of the impacted.
  * 2) Copy MCE# error bank to impacted DOM node list,
-      for vMCE# MSRs virtualization
-*/
-
+ *    for vMCE# MSRs virtualization
+ */
 int fill_vmsr_data(struct mcinfo_bank *mc_bank, struct domain *d,
-        uint64_t gstatus) {
+                   uint64_t gstatus) {
     struct bank_entry *entry;
 
     /* This error bank impacts one domain, we need to fill domain related
      * data for vMCE MSRs virtualization and vMCE# injection */
-    if (mc_bank->mc_domid != (uint16_t)~0) {
-        /* For HVM guest, Only when first vMCE is consumed by HVM guest 
successfully,
-         * will we generete another node and inject another vMCE
+    if ( mc_bank->mc_domid != (uint16_t)~0 )
+    {
+        /* For HVM guest, Only when first vMCE is consumed by HVM guest
+         * successfully, will we generete another node and inject another vMCE.
          */
-        if ( (d->is_hvm) && (d->arch.vmca_msrs->nr_injection > 0) )
+        if ( d->is_hvm && (dom_vmce(d)->nr_injection > 0) )
         {
             mce_printk(MCE_QUIET, "MCE: HVM guest has not handled previous"
-                        " vMCE yet!\n");
+                       " vMCE yet!\n");
             return -1;
         }
+
         entry = alloc_bank_entry();
-        if (entry == NULL)
+        if ( entry == NULL )
             return -1;
 
         entry->mci_status = mc_bank->mc_status;
@@ -421,29 +401,31 @@ int fill_vmsr_data(struct mcinfo_bank *m
         entry->mci_misc = mc_bank->mc_misc;
         entry->bank = mc_bank->mc_bank;
 
-        spin_lock(&d->arch.vmca_msrs->lock);
+        spin_lock(&dom_vmce(d)->lock);
         /* New error Node, insert to the tail of the per_dom data */
-        list_add_tail(&entry->list, &d->arch.vmca_msrs->impact_header);
+        list_add_tail(&entry->list, &dom_vmce(d)->impact_header);
         /* Fill MSR global status */
-        d->arch.vmca_msrs->mcg_status = gstatus;
+        dom_vmce(d)->mcg_status = gstatus;
         /* New node impact the domain, need another vMCE# injection*/
-        d->arch.vmca_msrs->nr_injection++;
-        spin_unlock(&d->arch.vmca_msrs->lock);
+        dom_vmce(d)->nr_injection++;
+        spin_unlock(&dom_vmce(d)->lock);
 
         mce_printk(MCE_VERBOSE,"MCE: Found error @[BANK%d "
-                "status %"PRIx64" addr %"PRIx64" domid %d]\n ",
-                mc_bank->mc_bank, mc_bank->mc_status, mc_bank->mc_addr,
-                mc_bank->mc_domid);
-    }
+                   "status %"PRIx64" addr %"PRIx64" domid %d]\n ",
+                   mc_bank->mc_bank, mc_bank->mc_status, mc_bank->mc_addr,
+                   mc_bank->mc_domid);
+    }
+
     return 0;
 }
 
-int vmce_domain_inject(struct mcinfo_bank *bank, struct domain *d, struct 
mcinfo_global *global)
+int vmce_domain_inject(
+    struct mcinfo_bank *bank, struct domain *d, struct mcinfo_global *global)
 {
     int ret;
 
     ret = fill_vmsr_data(bank, d, global->mc_gstatus);
-    if (ret < 0)
+    if ( ret < 0 )
         return ret;
 
     return inject_vmce(d);
diff -r 24a00bbe7cc0 -r 26da9bb87405 xen/arch/x86/domain.c
--- a/xen/arch/x86/domain.c     Thu May 06 11:19:28 2010 +0100
+++ b/xen/arch/x86/domain.c     Thu May 06 11:39:09 2010 +0100
@@ -535,6 +535,7 @@ int arch_domain_create(struct domain *d,
 
  fail:
     d->is_dying = DOMDYING_dead;
+    vmce_destroy_msr(d);
     xfree(d->arch.pirq_irq);
     xfree(d->arch.irq_pirq);
     free_xenheap_page(d->shared_info);
@@ -562,6 +563,7 @@ void arch_domain_destroy(struct domain *
     if ( is_hvm_domain(d) )
         hvm_domain_destroy(d);
 
+    vmce_destroy_msr(d);
     pci_release_devices(d);
     free_domain_pirqs(d);
     if ( !is_idle_domain(d) )
diff -r 24a00bbe7cc0 -r 26da9bb87405 xen/common/domain.c
--- a/xen/common/domain.c       Thu May 06 11:19:28 2010 +0100
+++ b/xen/common/domain.c       Thu May 06 11:39:09 2010 +0100
@@ -625,8 +625,6 @@ static void complete_domain_destroy(stru
 
     xfree(d->pirq_mask);
     xfree(d->pirq_to_evtchn);
-    xfree(dom_vmce(d)->mci_ctl);
-    xfree(dom_vmce(d));
 
     xsm_free_security_domain(d);
     free_domain_struct(d);
diff -r 24a00bbe7cc0 -r 26da9bb87405 xen/include/asm-x86/mce.h
--- a/xen/include/asm-x86/mce.h Thu May 06 11:19:28 2010 +0100
+++ b/xen/include/asm-x86/mce.h Thu May 06 11:39:09 2010 +0100
@@ -27,10 +27,9 @@ struct domain_mca_msrs
     spinlock_t lock;
 };
 
-#define dom_vmce(x)   ((x)->arch.vmca_msrs)
-
 /* Guest vMCE MSRs virtualization */
 extern int vmce_init_msr(struct domain *d);
+extern void vmce_destroy_msr(struct domain *d);
 extern int vmce_wrmsr(uint32_t msr, uint64_t val);
 extern int vmce_rdmsr(uint32_t msr, uint64_t *val);
 #endif

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.