WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel] Add a x86_mcinfo_reserve() function, to reserve space fr

To: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>, Keir Fraser <keir.fraser@xxxxxxxxxxxxx>, Christoph Egger <Christoph.Egger@xxxxxxx>, "Frank.Vanderlinden@xxxxxxx" <Frank.Vanderlinden@xxxxxxx>
Subject: RE: [Xen-devel] Add a x86_mcinfo_reserve() function, to reserve space from mc_info
From: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Date: Fri, 16 Apr 2010 19:06:36 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "Ke, Liping" <liping.ke@xxxxxxxxx>
Delivery-date: Fri, 16 Apr 2010 04:10:21 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <789F9655DD1B8F43B48D77C5D30659731D797931@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <789F9655DD1B8F43B48D77C5D30659731D797931@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcrdU94vnR1m0ng2SGqebAFXCnysPwAAHhkA
Thread-topic: [Xen-devel] Add a x86_mcinfo_reserve() function, to reserve space from mc_info
Sorry press send too quickly.

This patch is on top of previous " Clean-up on MCA MSR virtualization and vMCE 
injection" patch, although there is no logic relationship.
Also add the subject.

--jyh

>-----Original Message-----
>From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>[mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Jiang, Yunhong
>Sent: Friday, April 16, 2010 6:59 PM
>To: Keir Fraser; Christoph Egger; Frank.Vanderlinden@xxxxxxx
>Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; Ke, Liping
>Subject: [Xen-devel] (no subject)
>
>Add a x86_mcinfo_reserve() function, to reserve space from mc_info.
>
>With this method, we don't need to collect bank and globalinformation to a
>local variable and do x86_mcinfo_add() to copy that information to mc_info.
>This avoid copy and also we can be aware earlier if there is enough space
>in the mc_info.
>
>Also extract function that get global/bank information to seperated
>function mca_init_bank/mca_init_global.
>
>It's meaningless to get the current information in mce context, keep it here
>but should be removed in future.
>
>Also a flag added to mc_info, to indicate some information is lost due to OOM.
>
>Signed-off-by: Jiang, Yunhong <yunhong.jiang@xxxxxxxxx>
>
>diff -r 4e1d1e56e8b9 -r 63632454170a xen/arch/x86/cpu/mcheck/mce.c
>--- a/xen/arch/x86/cpu/mcheck/mce.c    Fri Apr 16 18:00:23 2010 +0800
>+++ b/xen/arch/x86/cpu/mcheck/mce.c    Fri Apr 16 18:51:58 2010 +0800
>@@ -125,6 +125,88 @@ void mce_need_clearbank_register(mce_nee
>     mc_need_clearbank_scan = cbfunc;
> }
>
>+static struct mcinfo_bank *mca_init_bank(enum mca_source who,
>+                                         struct mc_info *mi, int bank)
>+{
>+      struct mcinfo_bank *mib;
>+      uint64_t addr=0, misc = 0;
>+
>+      if (!mi)
>+              return NULL;
>+
>+      mib = x86_mcinfo_reserve(mi, sizeof(struct mcinfo_bank));
>+      if (!mib)
>+      {
>+              mi->flags |= MCINFO_FLAGS_UNCOMPLETE;
>+              return NULL;
>+      }
>+
>+      memset(mib, 0, sizeof (struct mcinfo_bank));
>+      mca_rdmsrl(MSR_IA32_MC0_STATUS + bank * 4, mib->mc_status);
>+
>+      mib->common.type = MC_TYPE_BANK;
>+      mib->common.size = sizeof (struct mcinfo_bank);
>+      mib->mc_bank = bank;
>+
>+      addr = misc = 0;
>+      if (mib->mc_status & MCi_STATUS_MISCV)
>+              mca_rdmsrl(MSR_IA32_MC0_MISC + 4 * bank, mib->mc_misc);
>+
>+      if (mib->mc_status & MCi_STATUS_ADDRV)
>+      {
>+              mca_rdmsrl(MSR_IA32_MC0_ADDR + 4 * bank, mib->mc_addr);
>+
>+              if (mfn_valid(paddr_to_pfn(mib->mc_addr))) {
>+                      struct domain *d;
>+
>+                      d = maddr_get_owner(mib->mc_addr);
>+                      if (d != NULL && (who == MCA_POLLER ||
>+                                who == MCA_CMCI_HANDLER))
>+                              mib->mc_domid = d->domain_id;
>+              }
>+      }
>+
>+      if (who == MCA_CMCI_HANDLER) {
>+              mca_rdmsrl(MSR_IA32_MC0_CTL2 + bank, mib->mc_ctrl2);
>+              rdtscll(mib->mc_tsc);
>+      }
>+
>+      return mib;
>+}
>+
>+static int mca_init_global(uint32_t flags, struct mcinfo_global *mig)
>+{
>+      uint64_t status;
>+      int cpu_nr;
>+      struct vcpu *v = current;
>+      struct domain *d;
>+
>+      /* Set global information */
>+      memset(mig, 0, sizeof (struct mcinfo_global));
>+      mig->common.type = MC_TYPE_GLOBAL;
>+      mig->common.size = sizeof (struct mcinfo_global);
>+      mca_rdmsrl(MSR_IA32_MCG_STATUS, status);
>+      mig->mc_gstatus = status;
>+      mig->mc_domid = mig->mc_vcpuid = -1;
>+      mig->mc_flags = flags;
>+      cpu_nr = smp_processor_id();
>+      /* Retrieve detector information */
>+      x86_mc_get_cpu_info(cpu_nr, &mig->mc_socketid,
>+        &mig->mc_coreid, &mig->mc_core_threadid,
>+        &mig->mc_apicid, NULL, NULL, NULL);
>+
>+      /* This is really meaningless */
>+      if (v != NULL && ((d = v->domain) != NULL)) {
>+              mig->mc_domid = d->domain_id;
>+              mig->mc_vcpuid = v->vcpu_id;
>+      } else {
>+              mig->mc_domid = -1;
>+              mig->mc_vcpuid = -1;
>+      }
>+
>+      return 0;
>+}
>+
> /* Utility function to perform MCA bank telemetry readout and to push that
>  * telemetry towards an interested dom0 for logging and diagnosis.
>  * The caller - #MC handler or MCA poll function - must arrange that we
>@@ -139,64 +221,38 @@ mctelem_cookie_t mcheck_mca_logout(enum
> mctelem_cookie_t mcheck_mca_logout(enum mca_source who, cpu_banks_t
>bankmask,
>     struct mca_summary *sp, cpu_banks_t* clear_bank)
> {
>-      struct vcpu *v = current;
>-      struct domain *d;
>-      uint64_t gstatus, status, addr, misc;
>-      struct mcinfo_global mcg;       /* on stack */
>-      struct mcinfo_common *mic;
>-      struct mcinfo_global *mig;      /* on stack */
>+      uint64_t gstatus, status;
>+      struct mcinfo_global *mig = NULL;       /* on stack */
>       mctelem_cookie_t mctc = NULL;
>-      uint32_t uc = 0, pcc = 0, recover, need_clear = 1 ;
>+      uint32_t uc = 0, pcc = 0, recover, need_clear = 1, mc_flags = 0;
>       struct mc_info *mci = NULL;
>       mctelem_class_t which = MC_URGENT;      /* XXXgcc */
>-      unsigned int cpu_nr;
>       int errcnt = 0;
>       int i;
>       enum mca_extinfo cbret = MCA_EXTINFO_IGNORED;
>
>-      cpu_nr = smp_processor_id();
>-      BUG_ON(cpu_nr != v->processor);
>-
>       mca_rdmsrl(MSR_IA32_MCG_STATUS, gstatus);
>-
>-      memset(&mcg, 0, sizeof (mcg));
>-      mcg.common.type = MC_TYPE_GLOBAL;
>-      mcg.common.size = sizeof (mcg);
>-      if (v != NULL && ((d = v->domain) != NULL)) {
>-              mcg.mc_domid = d->domain_id;
>-              mcg.mc_vcpuid = v->vcpu_id;
>-      } else {
>-              mcg.mc_domid = -1;
>-              mcg.mc_vcpuid = -1;
>-      }
>-      mcg.mc_gstatus = gstatus;       /* MCG_STATUS */
>-
>       switch (who) {
>       case MCA_MCE_HANDLER:
>       case MCA_MCE_SCAN:
>-              mcg.mc_flags = MC_FLAG_MCE;
>+              mc_flags = MC_FLAG_MCE;
>               which = MC_URGENT;
>               break;
>
>       case MCA_POLLER:
>       case MCA_RESET:
>-              mcg.mc_flags = MC_FLAG_POLLED;
>+              mc_flags = MC_FLAG_POLLED;
>               which = MC_NONURGENT;
>               break;
>
>       case MCA_CMCI_HANDLER:
>-              mcg.mc_flags = MC_FLAG_CMCI;
>+              mc_flags = MC_FLAG_CMCI;
>               which = MC_NONURGENT;
>               break;
>
>       default:
>               BUG();
>       }
>-
>-      /* Retrieve detector information */
>-      x86_mc_get_cpu_info(cpu_nr, &mcg.mc_socketid,
>-          &mcg.mc_coreid, &mcg.mc_core_threadid,
>-          &mcg.mc_apicid, NULL, NULL, NULL);
>
>       /* If no mc_recovery_scan callback handler registered,
>        * this error is not recoverable
>@@ -204,7 +260,7 @@ mctelem_cookie_t mcheck_mca_logout(enum
>       recover = (mc_recoverable_scan)? 1: 0;
>
>       for (i = 0; i < 32 && i < nr_mce_banks; i++) {
>-              struct mcinfo_bank mcb;         /* on stack */
>+              struct mcinfo_bank *mib;                /* on stack */
>
>               /* Skip bank if corresponding bit in bankmask is clear */
>               if (!test_bit(i, bankmask))
>@@ -227,17 +283,16 @@ mctelem_cookie_t mcheck_mca_logout(enum
>                * a poller;  this can fail (for example dom0 may not
>                * yet have consumed past telemetry). */
>               if (errcnt == 0) {
>-                      if ((mctc = mctelem_reserve(which)) != NULL) {
>+                      if ( (mctc = mctelem_reserve(which)) != NULL ) {
>                               mci = mctelem_dataptr(mctc);
>                               mcinfo_clear(mci);
>+                              mig = (struct mcinfo_global*)x86_mcinfo_reserve
>+                                (mci, sizeof(struct mcinfo_global));
>+                              /* mc_info should at least hold up the global 
>information */
>+                              ASSERT(mig);
>+                              mca_init_global(mc_flags, mig);
>                       }
>               }
>-
>-              memset(&mcb, 0, sizeof (mcb));
>-              mcb.common.type = MC_TYPE_BANK;
>-              mcb.common.size = sizeof (mcb);
>-              mcb.mc_bank = i;
>-              mcb.mc_status = status;
>
>               /* form a mask of which banks have logged uncorrected errors */
>               if ((status & MCi_STATUS_UC) != 0)
>@@ -252,37 +307,7 @@ mctelem_cookie_t mcheck_mca_logout(enum
>                 */
>                       recover = mc_recoverable_scan(status);
>
>-              addr = misc = 0;
>-
>-              if (status & MCi_STATUS_ADDRV) {
>-                      mca_rdmsrl(MSR_IA32_MC0_ADDR + 4 * i, addr);
>-                      if (mfn_valid(paddr_to_pfn(addr))) {
>-                              d = maddr_get_owner(addr);
>-                              if (d != NULL && (who == MCA_POLLER ||
>-                                  who == MCA_CMCI_HANDLER))
>-                                      mcb.mc_domid = d->domain_id;
>-                      }
>-              }
>-
>-              if (status & MCi_STATUS_MISCV)
>-                      mca_rdmsrl(MSR_IA32_MC0_MISC + 4 * i, misc);
>-
>-              mcb.mc_addr = addr;
>-              mcb.mc_misc = misc;
>-
>-              if (who == MCA_CMCI_HANDLER) {
>-                      mca_rdmsrl(MSR_IA32_MC0_CTL2 + i, mcb.mc_ctrl2);
>-                      rdtscll(mcb.mc_tsc);
>-              }
>-
>-              /* Increment the error count;  if this is the first bank
>-               * with a valid error then add the global info to the mcinfo. */
>-              if (errcnt++ == 0 && mci != NULL)
>-                      x86_mcinfo_add(mci, &mcg);
>-
>-              /* Add the bank data */
>-              if (mci != NULL)
>-                      x86_mcinfo_add(mci, &mcb);
>+              mib = mca_init_bank(who, mci, i);
>
>               if (mc_callback_bank_extended && cbret != MCA_EXTINFO_GLOBAL) {
>                       cbret = mc_callback_bank_extended(mci, i, status);
>@@ -298,12 +323,8 @@ mctelem_cookie_t mcheck_mca_logout(enum
>               wmb();
>       }
>
>-      if (mci != NULL && errcnt > 0) {
>-              x86_mcinfo_lookup(mic, mci, MC_TYPE_GLOBAL);
>-              mig = container_of(mic, struct mcinfo_global, common);
>-              if (mic == NULL)
>-                      ;
>-              else if (pcc)
>+      if (mig && errcnt > 0) {
>+              if (pcc)
>                       mig->mc_flags |= MC_FLAG_UNCORRECTABLE;
>               else if (uc)
>                       mig->mc_flags |= MC_FLAG_RECOVERABLE;
>@@ -758,13 +779,12 @@ static void mcinfo_clear(struct mc_info
>       x86_mcinfo_nentries(mi) = 0;
> }
>
>-int x86_mcinfo_add(struct mc_info *mi, void *mcinfo)
>+void *x86_mcinfo_reserve(struct mc_info *mi, int size)
> {
>       int i;
>       unsigned long end1, end2;
>-      struct mcinfo_common *mic, *mic_base, *mic_index;
>-
>-      mic = (struct mcinfo_common *)mcinfo;
>+      struct mcinfo_common *mic_base, *mic_index;
>+
>       mic_index = mic_base = x86_mcinfo_first(mi);
>
>       /* go to first free entry */
>@@ -774,16 +794,35 @@ int x86_mcinfo_add(struct mc_info *mi, v
>
>       /* check if there is enough size */
>       end1 = (unsigned long)((uint8_t *)mic_base + sizeof(struct mc_info));
>-      end2 = (unsigned long)((uint8_t *)mic_index + mic->size);
>+      end2 = (unsigned long)((uint8_t *)mic_index + size);
>
>       if (end1 < end2)
>-              return x86_mcerr("mcinfo_add: no more sparc", -ENOSPC);
>+      {
>+              mce_printk(MCE_CRITICAL,
>+                      "mcinfo_add: No space left in mc_info\n");
>+              return NULL;
>+      }
>
>       /* there's enough space. add entry. */
>-      memcpy(mic_index, mic, mic->size);
>       x86_mcinfo_nentries(mi)++;
>
>-      return 0;
>+    return mic_index;
>+}
>+
>+void *x86_mcinfo_add(struct mc_info *mi, void *mcinfo)
>+{
>+      struct mcinfo_common *mic, *buf;
>+
>+      mic = (struct mcinfo_common *)mcinfo;
>+      buf = x86_mcinfo_reserve(mi, mic->size);
>+
>+      if ( !buf )
>+              mce_printk(MCE_CRITICAL,
>+                      "mcinfo_add: No space left in mc_info\n");
>+      else
>+              memcpy(buf, mic, mic->size);
>+
>+      return buf;
> }
>
> /* Dump machine check information in a format,
>diff -r 4e1d1e56e8b9 -r 63632454170a xen/arch/x86/cpu/mcheck/mce.h
>--- a/xen/arch/x86/cpu/mcheck/mce.h    Fri Apr 16 18:00:23 2010 +0800
>+++ b/xen/arch/x86/cpu/mcheck/mce.h    Fri Apr 16 18:51:58 2010 +0800
>@@ -161,7 +161,8 @@ typedef enum mca_extinfo (*x86_mce_callb
>     (struct mc_info *, uint16_t, uint64_t);
> extern void x86_mce_callback_register(x86_mce_callback_t);
>
>-int x86_mcinfo_add(struct mc_info *mi, void *mcinfo);
>+void *x86_mcinfo_add(struct mc_info *mi, void *mcinfo);
>+void *x86_mcinfo_reserve(struct mc_info *mi, int size);
> void x86_mcinfo_dump(struct mc_info *mi);
>
> int fill_vmsr_data(struct mcinfo_bank *mc_bank, struct domain *d,
>diff -r 4e1d1e56e8b9 -r 63632454170a xen/include/public/arch-x86/xen-mca.h
>--- a/xen/include/public/arch-x86/xen-mca.h    Fri Apr 16 18:00:23 2010 +0800
>+++ b/xen/include/public/arch-x86/xen-mca.h    Fri Apr 16 18:51:58 2010 +0800
>@@ -233,10 +233,11 @@ struct mcinfo_recovery
> #define MCINFO_HYPERCALLSIZE  1024
> #define MCINFO_MAXSIZE                768
>
>+#define MCINFO_FLAGS_UNCOMPLETE 0x1
> struct mc_info {
>     /* Number of mcinfo_* entries in mi_data */
>     uint32_t mi_nentries;
>-    uint32_t _pad0;
>+    uint32_t flags;
>     uint64_t mi_data[(MCINFO_MAXSIZE - 1) / 8];
> };
> typedef struct mc_info mc_info_t;
>


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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] (no subject), Jiang, Yunhong
    • RE: [Xen-devel] Add a x86_mcinfo_reserve() function, to reserve space from mc_info, Jiang, Yunhong <=