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

Re: [Xen-devel] [PATCH] Xen/MCE: adjust for future new vMCE model



>>> On 05.07.12 at 05:30, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> Jan, I update according to your comments.

Some of them you did address, but I see that the per-domain
mci_ctl[] array is still present. What's the point?

Jan

> =====================
> Xen/MCE: adjust for future new vMCE model
> 
> Recently we designed a new vMCE model, and would implement later.
> 
> In order not to block live migration from current vMCE to future new vMCE,
> we do some middle work in this patch, basically it does minimal adjustment, 
> including
> 1. remove MCG_CTL;
> 2. stick all 1's to MCi_CTL;
> 3. set MCG_CTL_P=0 and 2 banks at MCG_CAP;
> 
> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
> 
> diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/mce.c
> --- a/xen/arch/x86/cpu/mcheck/mce.c   Wed Jun 27 09:36:43 2012 +0200
> +++ b/xen/arch/x86/cpu/mcheck/mce.c   Thu Jul 05 19:21:16 2012 +0800
> @@ -843,8 +843,6 @@
>  
>      mctelem_init(sizeof(struct mc_info));
>  
> -    vmce_init(c);
> -
>      /* Turn on MCE now */
>      set_in_cr4(X86_CR4_MCE);
>  
> diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/mce.h
> --- a/xen/arch/x86/cpu/mcheck/mce.h   Wed Jun 27 09:36:43 2012 +0200
> +++ b/xen/arch/x86/cpu/mcheck/mce.h   Thu Jul 05 19:21:16 2012 +0800
> @@ -170,8 +170,6 @@
>  int inject_vmce(struct domain *d);
>  int vmce_domain_inject(struct mcinfo_bank *bank, struct domain *d, struct 
> mcinfo_global *global);
>  
> -extern int vmce_init(struct cpuinfo_x86 *c);
> -
>  static inline int mce_vendor_bank_msr(const struct vcpu *v, uint32_t msr)
>  {
>      if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> diff -r 4f92bdf3370c xen/arch/x86/cpu/mcheck/vmce.c
> --- a/xen/arch/x86/cpu/mcheck/vmce.c  Wed Jun 27 09:36:43 2012 +0200
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c  Thu Jul 05 19:21:16 2012 +0800
> @@ -19,36 +19,42 @@
>  #include "mce.h"
>  #include "x86_mca.h"
>  
> +/*
> + * Emulalte 2 banks for guest
> + * Bank0: reserved for 'bank0 quirk' occur at some very old processors:
> + *   1). Intel cpu whose family-model value < 06-1A;
> + *   2). AMD K7
> + * Bank1: used to transfer error info to guest
> + */
> +#define GUEST_BANK_NUM 2
> +
>  #define dom_vmce(x)   ((x)->arch.vmca_msrs)
>  
>  static uint64_t __read_mostly g_mcg_cap;
>  
> -/* Real value in physical CTL MSR */
> -static uint64_t __read_mostly h_mcg_ctl;
> -static uint64_t *__read_mostly h_mci_ctrl;
> -
>  int vmce_init_msr(struct domain *d)
>  {
>      dom_vmce(d) = xmalloc(struct domain_mca_msrs);
>      if ( !dom_vmce(d) )
>          return -ENOMEM;
>  
> -    dom_vmce(d)->mci_ctl = xmalloc_array(uint64_t, nr_mce_banks);
> +    dom_vmce(d)->mci_ctl = xmalloc_array(uint64_t, GUEST_BANK_NUM);
>      if ( !dom_vmce(d)->mci_ctl )
>      {
>          xfree(dom_vmce(d));
>          return -ENOMEM;
>      }
>      memset(dom_vmce(d)->mci_ctl, ~0,
> -           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl));
> +           GUEST_BANK_NUM * sizeof(*dom_vmce(d)->mci_ctl));
>  
>      dom_vmce(d)->mcg_status = 0x0;
> -    dom_vmce(d)->mcg_ctl = ~(uint64_t)0x0;
>      dom_vmce(d)->nr_injection = 0;
>  
>      INIT_LIST_HEAD(&dom_vmce(d)->impact_header);
>      spin_lock_init(&dom_vmce(d)->lock);
>  
> +    g_mcg_cap = MCG_TES_P | MCG_SER_P | GUEST_BANK_NUM;
> +
>      return 0;
>  }
>  
> @@ -68,7 +74,7 @@
>  
>  int vmce_restore_vcpu(struct vcpu *v, uint64_t caps)
>  {
> -    if ( caps & ~g_mcg_cap & ~MCG_CAP_COUNT & ~MCG_CTL_P )
> +    if ( caps & ~g_mcg_cap & ~MCG_CAP_COUNT )
>      {
>          dprintk(XENLOG_G_ERR, "%s restore: unsupported MCA capabilities"
>                  " %#" PRIx64 " for d%d:v%u (supported: %#Lx)\n",
> @@ -77,6 +83,12 @@
>          return -EPERM;
>      }
>  
> +    if ( (caps & MCG_CAP_COUNT) != GUEST_BANK_NUM )
> +    {
> +        dprintk(XENLOG_G_ERR, "Bank number inconsistency\n");
> +        return -EPERM;
> +    }
> +
>      v->arch.mcg_cap = caps;
>      return 0;
>  }
> @@ -93,9 +105,9 @@
>      switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>      {
>      case MSR_IA32_MC0_CTL:
> -        if ( bank < nr_mce_banks )
> -            *val = vmce->mci_ctl[bank] &
> -                   (h_mci_ctrl ? h_mci_ctrl[bank] : ~0UL);
> +        BUG_ON( bank >= GUEST_BANK_NUM );
> +        BUG_ON( vmce->mci_ctl[bank] != ~0UL );
> +        *val = ~0UL;
>          mce_printk(MCE_VERBOSE, "MCE: rdmsr MC%u_CTL 0x%"PRIx64"\n",
>                     bank, *val);
>          break;
> @@ -187,11 +199,9 @@
>                     *val);
>          break;
>      case MSR_IA32_MCG_CTL:
> -        /* Always 0 if no CTL support */
> -        if ( cur->arch.mcg_cap & MCG_CTL_P )
> -            *val = vmce->mcg_ctl & h_mcg_ctl;
> -        mce_printk(MCE_VERBOSE, "MCE: rdmsr MCG_CTL 0x%"PRIx64"\n",
> -                   *val);
> +        BUG_ON( cur->arch.mcg_cap & MCG_CTL_P );
> +        mce_printk(MCE_QUIET, "MCE: no MCG_CTL\n");
> +        ret = -1;
>          break;
>      default:
>          ret = mce_bank_msr(cur, msr) ? bank_mce_rdmsr(cur, msr, val) : 0;
> @@ -220,8 +230,11 @@
>      switch ( msr & (MSR_IA32_MC0_CTL | 3) )
>      {
>      case MSR_IA32_MC0_CTL:
> -        if ( bank < nr_mce_banks )
> -            vmce->mci_ctl[bank] = val;
> +        BUG_ON( bank >= GUEST_BANK_NUM );
> +        /* 
> +         * if guest crazy clear any bit of MCi_CTL,
> +         * treat it as not implement and ignore write change it.
> +         */
>          break;
>      case MSR_IA32_MC0_STATUS:
>          if ( entry && (entry->bank == bank) )
> @@ -305,7 +318,9 @@
>      switch ( msr )
>      {
>      case MSR_IA32_MCG_CTL:
> -        vmce->mcg_ctl = val;
> +        BUG_ON( cur->arch.mcg_cap & MCG_CTL_P );
> +        mce_printk(MCE_QUIET, "MCE: no MCG_CTL\n");
> +        ret = -1;
>          break;
>      case MSR_IA32_MCG_STATUS:
>          vmce->mcg_status = val;
> @@ -520,52 +535,6 @@
>  }
>  #endif
>  
> -int vmce_init(struct cpuinfo_x86 *c)
> -{
> -    u64 value;
> -    unsigned int i;
> -
> -    if ( !h_mci_ctrl )
> -    {
> -        h_mci_ctrl = xmalloc_array(uint64_t, nr_mce_banks);
> -        if (!h_mci_ctrl)
> -        {
> -            dprintk(XENLOG_INFO, "Failed to alloc h_mci_ctrl\n");
> -            return -ENOMEM;
> -        }
> -        /* Don't care banks before firstbank */
> -        memset(h_mci_ctrl, ~0,
> -               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
> -        for (i = firstbank; i < nr_mce_banks; i++)
> -            rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
> -    }
> -
> -    rdmsrl(MSR_IA32_MCG_CAP, value);
> -    /* For Guest vMCE usage */
> -    g_mcg_cap = value & (MCG_CAP_COUNT | MCG_CTL_P | MCG_TES_P | MCG_SER_P);
> -    if (value & MCG_CTL_P)
> -        rdmsrl(MSR_IA32_MCG_CTL, h_mcg_ctl);
> -
> -    return 0;
> -}
> -
> -static int mca_ctl_conflict(struct mcinfo_bank *bank, struct domain *d)
> -{
> -    int bank_nr;
> -
> -    if ( !bank || !d || !h_mci_ctrl )
> -        return 1;
> -
> -    /* Will MCE happen in host if If host mcg_ctl is 0? */
> -    if ( ~d->arch.vmca_msrs->mcg_ctl & h_mcg_ctl )
> -        return 1;
> -
> -    bank_nr = bank->mc_bank;
> -    if (~d->arch.vmca_msrs->mci_ctl[bank_nr] & h_mci_ctrl[bank_nr] )
> -        return 1;
> -    return 0;
> -}
> -
>  static int is_hvm_vmce_ready(struct mcinfo_bank *bank, struct domain *d)
>  {
>      struct vcpu *v;
> @@ -619,14 +588,6 @@
>      if (no_vmce)
>          return 0;
>  
> -    /* Guest has different MCE ctl value setting */
> -    if (mca_ctl_conflict(bank, d))
> -    {
> -        dprintk(XENLOG_WARNING,
> -          "No vmce, guest has different mca control setting\n");
> -        return 0;
> -    }
> -
>      return 1;
>  }
>  
> diff -r 4f92bdf3370c xen/include/asm-x86/mce.h
> --- a/xen/include/asm-x86/mce.h       Wed Jun 27 09:36:43 2012 +0200
> +++ b/xen/include/asm-x86/mce.h       Thu Jul 05 19:21:16 2012 +0800
> @@ -16,7 +16,6 @@
>  struct domain_mca_msrs
>  {
>      /* Guest should not change below values after DOM boot up */
> -    uint64_t mcg_ctl;
>      uint64_t mcg_status;
>      uint64_t *mci_ctl;
>      uint16_t nr_injection;



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


 


Rackspace

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