diff mbox

[17/19] x86/vmce, tools/libxl: expose LMCE capability in guest MSR_IA32_MCG_CAP

Message ID 20170217063936.13208-18-haozhong.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Haozhong Zhang Feb. 17, 2017, 6:39 a.m. UTC
If LMCE is supported by host and "lmce = 1" is present in xl config, the
LMCE capability will be exposed in guest MSR_IA32_MCG_CAP. By default,
LMCE is not exposed to guest so as to keep the backwards migration
compatibility.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Christoph Egger <chegger@amazon.de>
Cc: Liu Jinsong <jinsong.liu@alibaba-inc.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 docs/man/xl.cfg.pod.5.in        | 18 ++++++++++++++++++
 tools/libxl/libxl_create.c      |  1 +
 tools/libxl/libxl_dom.c         |  2 ++
 tools/libxl/libxl_types.idl     |  1 +
 tools/libxl/xl_cmdimpl.c        |  3 +++
 xen/arch/x86/cpu/mcheck/vmce.c  | 14 +++++++++++++-
 xen/arch/x86/hvm/hvm.c          |  7 +++++++
 xen/include/asm-x86/mce.h       |  1 +
 xen/include/public/hvm/params.h |  5 ++++-
 9 files changed, 50 insertions(+), 2 deletions(-)

Comments

Wei Liu Feb. 20, 2017, 12:32 p.m. UTC | #1
On Fri, Feb 17, 2017 at 02:39:34PM +0800, Haozhong Zhang wrote:
> If LMCE is supported by host and "lmce = 1" is present in xl config, the
> LMCE capability will be exposed in guest MSR_IA32_MCG_CAP. By default,
> LMCE is not exposed to guest so as to keep the backwards migration
> compatibility.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Sorry for this stupid question: what does L in LMCE stand for? Googling
LMCE doesn't return useful result.

>  
>  int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index a612d1f..3cb0d9a 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -550,6 +550,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                         ("serial_list",      libxl_string_list),
>                                         ("rdm", libxl_rdm_reserve),
>                                         ("rdm_mem_boundary_memkb", MemKB),
> +                                       ("lmce",             libxl_defbool),

You also need to add LIBXL_HAVE_LMCE to libxl.h.

Wei.
Jan Beulich Feb. 20, 2017, 12:38 p.m. UTC | #2
>>> On 20.02.17 at 13:32, <wei.liu2@citrix.com> wrote:
> On Fri, Feb 17, 2017 at 02:39:34PM +0800, Haozhong Zhang wrote:
>> If LMCE is supported by host and "lmce = 1" is present in xl config, the
>> LMCE capability will be exposed in guest MSR_IA32_MCG_CAP. By default,
>> LMCE is not exposed to guest so as to keep the backwards migration
>> compatibility.
>> 
>> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Sorry for this stupid question: what does L in LMCE stand for? Googling
> LMCE doesn't return useful result.

I think it's "local" (as opposed to the broadcasting to all (v)CPUs).

Jan
Wei Liu Feb. 20, 2017, 2:12 p.m. UTC | #3
On Mon, Feb 20, 2017 at 05:38:53AM -0700, Jan Beulich wrote:
> >>> On 20.02.17 at 13:32, <wei.liu2@citrix.com> wrote:
> > On Fri, Feb 17, 2017 at 02:39:34PM +0800, Haozhong Zhang wrote:
> >> If LMCE is supported by host and "lmce = 1" is present in xl config, the
> >> LMCE capability will be exposed in guest MSR_IA32_MCG_CAP. By default,
> >> LMCE is not exposed to guest so as to keep the backwards migration
> >> compatibility.
> >> 
> >> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> > 
> > Sorry for this stupid question: what does L in LMCE stand for? Googling
> > LMCE doesn't return useful result.
> 
> I think it's "local" (as opposed to the broadcasting to all (v)CPUs).
> 

Actually Haozhong already mentioned that in the cover letter. My bad.

> Jan
>
Haozhong Zhang Feb. 20, 2017, 11:55 p.m. UTC | #4
On 02/20/17 12:32 +0000, Wei Liu wrote:
> On Fri, Feb 17, 2017 at 02:39:34PM +0800, Haozhong Zhang wrote:
> > If LMCE is supported by host and "lmce = 1" is present in xl config, the
> > LMCE capability will be exposed in guest MSR_IA32_MCG_CAP. By default,
> > LMCE is not exposed to guest so as to keep the backwards migration
> > compatibility.
> > 
> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> 
> Sorry for this stupid question: what does L in LMCE stand for? Googling
> LMCE doesn't return useful result.
>

Local

> >  
> >  int libxl__build_pre(libxl__gc *gc, uint32_t domid,
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index a612d1f..3cb0d9a 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -550,6 +550,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> >                                         ("serial_list",      libxl_string_list),
> >                                         ("rdm", libxl_rdm_reserve),
> >                                         ("rdm_mem_boundary_memkb", MemKB),
> > +                                       ("lmce",             libxl_defbool),
> 
> You also need to add LIBXL_HAVE_LMCE to libxl.h.
> 

will do

Thanks,
Haozhong
Jan Beulich Feb. 22, 2017, 3:55 p.m. UTC | #5
>>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote:
> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> @@ -74,7 +74,7 @@ int vmce_restore_vcpu(struct vcpu *v, const struct 
> hvm_vmce_vcpu *ctxt)
>      unsigned long guest_mcg_cap;
>  
>      if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
> -        guest_mcg_cap = INTEL_GUEST_MCG_CAP;
> +        guest_mcg_cap = INTEL_GUEST_MCG_CAP | (lmce_support ? MCG_LMCE_P : 0);
>      else
>          guest_mcg_cap = AMD_GUEST_MCG_CAP;
>  

Is it really a problem to restore a guest that has LMCE enabled on
an LMCE-incapable host? I.e. doesn't the guest need to be aware
of MCEs with the local bit clear anyway?

> @@ -4185,6 +4186,12 @@ static int hvmop_set_param(
>          }
>          d->arch.x87_fip_width = a.value;
>          break;
> +    case HVM_PARAM_LMCE:
> +        if ( a.value > 1 )
> +            rc = -EINVAL;
> +        else if ( a.value == 1 )
> +            rc = vmce_enable_lmce(d);
> +        break;

If you named the param slightly differently, and if you introduced
a #define for the one bit you care about, it could be ready for use
for further future flags right away.

Jan
Haozhong Zhang Feb. 23, 2017, 5:07 a.m. UTC | #6
On 02/22/17 08:55 -0700, Jan Beulich wrote:
> >>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote:
> > --- a/xen/arch/x86/cpu/mcheck/vmce.c
> > +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> > @@ -74,7 +74,7 @@ int vmce_restore_vcpu(struct vcpu *v, const struct 
> > hvm_vmce_vcpu *ctxt)
> >      unsigned long guest_mcg_cap;
> >  
> >      if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
> > -        guest_mcg_cap = INTEL_GUEST_MCG_CAP;
> > +        guest_mcg_cap = INTEL_GUEST_MCG_CAP | (lmce_support ? MCG_LMCE_P : 0);
> >      else
> >          guest_mcg_cap = AMD_GUEST_MCG_CAP;
> >  
> 
> Is it really a problem to restore a guest that has LMCE enabled on
> an LMCE-incapable host? I.e. doesn't the guest need to be aware
> of MCEs with the local bit clear anyway?

There should be no problem. I'll change it to
-        guest_mcg_cap = INTEL_GUEST_MCG_CAP;
+        guest_mcg_cap = INTEL_GUEST_MCG_CAP | MCG_LMCE_P;

> 
> > @@ -4185,6 +4186,12 @@ static int hvmop_set_param(
> >          }
> >          d->arch.x87_fip_width = a.value;
> >          break;
> > +    case HVM_PARAM_LMCE:
> > +        if ( a.value > 1 )
> > +            rc = -EINVAL;
> > +        else if ( a.value == 1 )
> > +            rc = vmce_enable_lmce(d);
> > +        break;
> 
> If you named the param slightly differently, and if you introduced
> a #define for the one bit you care about, it could be ready for use
> for further future flags right away.
>

Yes, I'll rename it to HVM_PARAM_MCE_CAP and reserve bit 0 for LMCE capability.

Thanks,
Haozhong
diff mbox

Patch

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 46f9caf..1cdf372 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -2021,6 +2021,24 @@  natively or via hardware backwards compatibility support.
 
 =back
 
+=head3 Intel
+
+=over 4
+
+=item B<lmce=BOOLEAN>
+
+(HVM only) Enable/disable LMCE support for a HVM domain.
+
+=over 4
+
+=item B<default>
+
+Disabled.
+
+=back
+
+=back
+
 =head1 SEE ALSO
 
 =over 4
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e3bc257..381e5dc 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -324,6 +324,7 @@  int libxl__domain_build_info_setdefault(libxl__gc *gc,
         libxl_defbool_setdefault(&b_info->u.hvm.altp2m,             false);
         libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
         libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
+        libxl_defbool_setdefault(&b_info->u.hvm.lmce,               false);
 
         libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
         if (!libxl_defbool_val(b_info->u.hvm.spice.enable) &&
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index d519c8d..f04adf4 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -293,6 +293,8 @@  static void hvm_set_conf_params(xc_interface *handle, uint32_t domid,
                     libxl_defbool_val(info->u.hvm.nested_hvm));
     xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M,
                     libxl_defbool_val(info->u.hvm.altp2m));
+    xc_hvm_param_set(handle, domid, HVM_PARAM_LMCE,
+                     libxl_defbool_val(info->u.hvm.lmce));
 }
 
 int libxl__build_pre(libxl__gc *gc, uint32_t domid,
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index a612d1f..3cb0d9a 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -550,6 +550,7 @@  libxl_domain_build_info = Struct("domain_build_info",[
                                        ("serial_list",      libxl_string_list),
                                        ("rdm", libxl_rdm_reserve),
                                        ("rdm_mem_boundary_memkb", MemKB),
+                                       ("lmce",             libxl_defbool),
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 37ebdce..4ed8e3e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1698,6 +1698,9 @@  static void parse_config_data(const char *config_source,
 
         if (!xlu_cfg_get_long (config, "rdm_mem_boundary", &l, 0))
             b_info->u.hvm.rdm_mem_boundary_memkb = l * 1024;
+
+        xlu_cfg_get_defbool(config, "lmce", &b_info->u.hvm.lmce, 0);
+
         break;
     case LIBXL_DOMAIN_TYPE_PV:
     {
diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index 2a4d3f0..fa9b499 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -74,7 +74,7 @@  int vmce_restore_vcpu(struct vcpu *v, const struct hvm_vmce_vcpu *ctxt)
     unsigned long guest_mcg_cap;
 
     if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
-        guest_mcg_cap = INTEL_GUEST_MCG_CAP;
+        guest_mcg_cap = INTEL_GUEST_MCG_CAP | (lmce_support ? MCG_LMCE_P : 0);
     else
         guest_mcg_cap = AMD_GUEST_MCG_CAP;
 
@@ -519,3 +519,15 @@  int unmmap_broken_page(struct domain *d, mfn_t mfn, unsigned long gfn)
     return rc;
 }
 
+int vmce_enable_lmce(struct domain *d)
+{
+    struct vcpu *v;
+
+    if ( !lmce_support )
+        return -EINVAL;
+
+    for_each_vcpu(d, v)
+        v->arch.vmce.mcg_cap |= MCG_LMCE_P;
+
+    return 0;
+}
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 266f708..19389c0 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4007,6 +4007,7 @@  static int hvm_allow_set_param(struct domain *d,
     case HVM_PARAM_IOREQ_SERVER_PFN:
     case HVM_PARAM_NR_IOREQ_SERVER_PAGES:
     case HVM_PARAM_ALTP2M:
+    case HVM_PARAM_LMCE:
         if ( value != 0 && a->value != value )
             rc = -EEXIST;
         break;
@@ -4185,6 +4186,12 @@  static int hvmop_set_param(
         }
         d->arch.x87_fip_width = a.value;
         break;
+    case HVM_PARAM_LMCE:
+        if ( a.value > 1 )
+            rc = -EINVAL;
+        else if ( a.value == 1 )
+            rc = vmce_enable_lmce(d);
+        break;
     }
 
     if ( rc != 0 )
diff --git a/xen/include/asm-x86/mce.h b/xen/include/asm-x86/mce.h
index 525a9e8..f5a9ff9 100644
--- a/xen/include/asm-x86/mce.h
+++ b/xen/include/asm-x86/mce.h
@@ -38,6 +38,7 @@  extern int vmce_restore_vcpu(struct vcpu *, const struct hvm_vmce_vcpu *);
 extern int vmce_wrmsr(uint32_t msr, uint64_t val);
 extern int vmce_rdmsr(uint32_t msr, uint64_t *val);
 extern bool vmce_support_lmce(const struct vcpu *v);
+extern int vmce_enable_lmce(struct domain *d);
 
 extern unsigned int nr_mce_banks;
 
diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 3f54a49..6b6ecbe 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -253,6 +253,9 @@ 
  */
 #define HVM_PARAM_X87_FIP_WIDTH 36
 
-#define HVM_NR_PARAMS 37
+/* Boolean: Enable LMCE */
+#define HVM_PARAM_LMCE 37
+
+#define HVM_NR_PARAMS 38
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */