| Message ID | 20170317064614.23539-10-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 17.03.17 at 07:46, <haozhong.zhang@intel.com> wrote: > --- a/xen/include/public/hvm/params.h > +++ b/xen/include/public/hvm/params.h > @@ -259,6 +259,11 @@ > */ > #define HVM_PARAM_VM86_TSS_SIZED 37 > > -#define HVM_NR_PARAMS 38 > +/* Enable MCA capabilities. */ > +#define HVM_PARAM_MCA_CAP 38 > +#define HVMMCA_CAP_LMCE (1UL << 27) How come this is bit 27 rather than bit 0? Further, to match up with other, future definitions here, I think you mean 1ULL (or, to be precise, xen_mk_ullong()). > +#define HVMMCA_CAP_MASK HVMMCA_CAP_LMCE For both of these, please add XEN_ at the front (perhaps replacing HVM, or if not, add another underscore between HVM and MCA). Jan
Haozhong Zhang writes ("[PATCH v2 09/12] x86/vmce, tools/libxl: expose LMCE capability in guest MSR_IA32_MCG_CAP"):
> If LMCE is supported by host and ' mca_caps = [ "lmce" ] ' 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.
Excluse my ignorance, but I'm not sure what MCE, MCA and LMCE are.
I'm guessing MC = Machine Check ?
Is MCA "Machine Check Architecture" ? Is that precisely the machinery
for handling MCEs, or is it something else ?
I think we may need a bit of care in the user-facing interface to try
to make the names and docs comprehensible.
> +=head3 x86
> +
> +=over 4
> +
> +=item B<mca_caps=[ "CAP", "CAP", ... ]>
> +
> +(HVM only) Enable MCA capabilities besides default ones enabled
> +by Xen hypervisor for the HVM domain. "CAP" can be one in the
> +following list:
Would it be incorrect to replace references to "MCA" with "MCE" (eg,
changing "mca_caps" to "mce_caps" here) ?
Ian.
On 03/20/17 10:33 -0600, Jan Beulich wrote: > >>> On 17.03.17 at 07:46, <haozhong.zhang@intel.com> wrote: > > --- a/xen/include/public/hvm/params.h > > +++ b/xen/include/public/hvm/params.h > > @@ -259,6 +259,11 @@ > > */ > > #define HVM_PARAM_VM86_TSS_SIZED 37 > > > > -#define HVM_NR_PARAMS 38 > > +/* Enable MCA capabilities. */ > > +#define HVM_PARAM_MCA_CAP 38 > > +#define HVMMCA_CAP_LMCE (1UL << 27) > > How come this is bit 27 rather than bit 0? Further, to match up with > other, future definitions here, I think you mean 1ULL (or, to be > precise, xen_mk_ullong()). > I intended to use the same bit in MSR_IA32_MCG_CAP to clarify its semantic, but I realize it's unnecessary as I still need to introduce HVMMCA_CAP_* for each bit. Thanks, Haozhong
On 03/20/17 18:27 +0000, Ian Jackson wrote: > Haozhong Zhang writes ("[PATCH v2 09/12] x86/vmce, tools/libxl: expose LMCE capability in guest MSR_IA32_MCG_CAP"): > > If LMCE is supported by host and ' mca_caps = [ "lmce" ] ' 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. > > Excluse my ignorance, but I'm not sure what MCE, MCA and LMCE are. > I'm guessing MC = Machine Check ? > Yes. > Is MCA "Machine Check Architecture" ? Is that precisely the machinery > for handling MCEs, or is it something else ? Yes for both. And LMCE stands for 'Local Machine Check Error', which is only sent to the affected CPU, in contrast to the current ones on Intel CPU that are broadcast to all CPUs. > > I think we may need a bit of care in the user-facing interface to try > to make the names and docs comprehensible. > 'mca_caps' stands for capabilities of the machine check architecture present to guest. I choose this name because the existing intel_init_mca() uses "MCA capability" when it prints supported capabilities, and ... > > +=head3 x86 > > + > > +=over 4 > > + > > +=item B<mca_caps=[ "CAP", "CAP", ... ]> > > + > > +(HVM only) Enable MCA capabilities besides default ones enabled > > +by Xen hypervisor for the HVM domain. "CAP" can be one in the > > +following list: > > Would it be incorrect to replace references to "MCA" with "MCE" (eg, > changing "mca_caps" to "mce_caps" here) ? > ... besides for consistency with intel_init_mca(), Intel SDM uses "the information about the machine check architecture" (at the beginning of section "IA32_MCG_CAP MSR", Vol 3). 'mca_caps' currently is for capabilities indicated by IA32_MCG_CAP, so I think 'mca' is a better name than 'mce'. Thanks, Haozhong
On 03/21/17 15:29 +0800, Haozhong Zhang wrote: > On 03/20/17 18:27 +0000, Ian Jackson wrote: > > Haozhong Zhang writes ("[PATCH v2 09/12] x86/vmce, tools/libxl: expose LMCE capability in guest MSR_IA32_MCG_CAP"): [..] > > Is MCA "Machine Check Architecture" ? Is that precisely the machinery > > for handling MCEs, or is it something else ? More precisely, MCA is "the mechanism for detecting and reporting hardware (machine) errors", according to Intel SDM Vol 3, Section "MACHINE-CHECK ARCHITECTURE". Haozhong
>>> On 20.03.17 at 19:27, <ian.jackson@eu.citrix.com> wrote: > Haozhong Zhang writes ("[PATCH v2 09/12] x86/vmce, tools/libxl: expose LMCE > capability in guest MSR_IA32_MCG_CAP"): >> +=head3 x86 >> + >> +=over 4 >> + >> +=item B<mca_caps=[ "CAP", "CAP", ... ]> >> + >> +(HVM only) Enable MCA capabilities besides default ones enabled >> +by Xen hypervisor for the HVM domain. "CAP" can be one in the >> +following list: > > Would it be incorrect to replace references to "MCA" with "MCE" (eg, > changing "mca_caps" to "mce_caps" here) ? MCE (Machine Check Exception) is only part of MCA, so I don't think the terms can be used interchangeably. Namely I think mce_caps would be wrong. Jan
On Fri, Mar 17, 2017 at 02:46:11PM +0800, Haozhong Zhang wrote: > If LMCE is supported by host and ' mca_caps = [ "lmce" ] ' 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> [...] > +static int hvm_set_mca_capabilities(libxl__gc *gc, uint32_t domid, > + libxl_domain_build_info *const info) > +{ > + int i, rc = 0; > + const libxl_string_list xl_caps = info->u.hvm.mca_caps; > + unsigned long caps = 0; > + > + if (!xl_caps) > + return 0; > + > + for (i = 0; xl_caps[i] != NULL; i++) { > + if (!strcmp(xl_caps[i], "lmce")) > + caps |= HVMMCA_CAP_LMCE; > + else { > + LOG(ERROR, "Unsupported MCA capability %s", xl_caps[i]); > + rc = ERROR_FAIL; > + break; > + } > + } > + This is not how we normally do this. Parsing should normally be done in xl. Is there any particular reason you chose to parse here? > + if (!rc) > + rc = xc_hvm_param_set(CTX->xch, domid, HVM_PARAM_MCA_CAP, caps); > + > + return rc; > +} > #endif > > static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, > @@ -438,6 +464,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, > rc = hvm_set_viridian_features(gc, domid, info); > if (rc) > return rc; > + > + rc = hvm_set_mca_capabilities(gc, domid, info); > + if (rc) > + return rc; > #endif > } > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index a612d1f..71c6c33 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), > + ("mca_caps", libxl_string_list), You need to add a LIBXL_HAVE_MCA_CAPS to libxl.h Wei.
diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in index 505c111..d75dcd2 100644 --- a/docs/man/xl.cfg.pod.5.in +++ b/docs/man/xl.cfg.pod.5.in @@ -2021,6 +2021,30 @@ natively or via hardware backwards compatibility support. =back +=head3 x86 + +=over 4 + +=item B<mca_caps=[ "CAP", "CAP", ... ]> + +(HVM only) Enable MCA capabilities besides default ones enabled +by Xen hypervisor for the HVM domain. "CAP" can be one in the +following list: + +=over 4 + +=item B<"lmce"> + +Intel local MCE + +=item B<default> + +No MCA capabilities in above list are enabled. + +=back + +=back + =head1 SEE ALSO =over 4 diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index d519c8d..91b2f08 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -275,6 +275,32 @@ err: libxl_bitmap_dispose(&enlightenments); return ERROR_FAIL; } + +static int hvm_set_mca_capabilities(libxl__gc *gc, uint32_t domid, + libxl_domain_build_info *const info) +{ + int i, rc = 0; + const libxl_string_list xl_caps = info->u.hvm.mca_caps; + unsigned long caps = 0; + + if (!xl_caps) + return 0; + + for (i = 0; xl_caps[i] != NULL; i++) { + if (!strcmp(xl_caps[i], "lmce")) + caps |= HVMMCA_CAP_LMCE; + else { + LOG(ERROR, "Unsupported MCA capability %s", xl_caps[i]); + rc = ERROR_FAIL; + break; + } + } + + if (!rc) + rc = xc_hvm_param_set(CTX->xch, domid, HVM_PARAM_MCA_CAP, caps); + + return rc; +} #endif static void hvm_set_conf_params(xc_interface *handle, uint32_t domid, @@ -438,6 +464,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, rc = hvm_set_viridian_features(gc, domid, info); if (rc) return rc; + + rc = hvm_set_mca_capabilities(gc, domid, info); + if (rc) + return rc; #endif } diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index a612d1f..71c6c33 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), + ("mca_caps", libxl_string_list), ])), ("pv", Struct(None, [("kernel", string), ("slack_memkb", MemKB), diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 1ef0c27..11a9f51 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1084,6 +1084,10 @@ 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_list_as_string_list(config, "mca_caps", + &b_info->u.hvm.mca_caps, false); + 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 994a50e..5dc790f 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 | MCG_LMCE_P; else guest_mcg_cap = AMD_GUEST_MCG_CAP; @@ -546,3 +546,20 @@ int unmmap_broken_page(struct domain *d, mfn_t mfn, unsigned long gfn) return rc; } +int vmce_enable_mca_cap(struct domain *d, uint64_t cap) +{ + struct vcpu *v; + + if ( cap & ~HVMMCA_CAP_MASK ) + return -EINVAL; + + if ( cap & HVMMCA_CAP_LMCE ) + { + 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 ccfae4f..0007e3d 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -4045,6 +4045,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_MCA_CAP: if ( value != 0 && a->value != value ) rc = -EEXIST; break; @@ -4257,6 +4258,10 @@ static int hvmop_set_param( (0x10000 / 8) + 1) << 32); a.value |= VM86_TSS_UPDATED; break; + + case HVM_PARAM_MCA_CAP: + rc = vmce_enable_mca_cap(d, a.value); + break; } if ( rc != 0 ) diff --git a/xen/include/asm-x86/mce.h b/xen/include/asm-x86/mce.h index dee66b3..70eb50c 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_mca_cap(struct domain *d, uint64_t cap); extern unsigned int nr_mce_banks; diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h index 58c8478..266c123 100644 --- a/xen/include/public/hvm/params.h +++ b/xen/include/public/hvm/params.h @@ -259,6 +259,11 @@ */ #define HVM_PARAM_VM86_TSS_SIZED 37 -#define HVM_NR_PARAMS 38 +/* Enable MCA capabilities. */ +#define HVM_PARAM_MCA_CAP 38 +#define HVMMCA_CAP_LMCE (1UL << 27) +#define HVMMCA_CAP_MASK HVMMCA_CAP_LMCE + +#define HVM_NR_PARAMS 39 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
If LMCE is supported by host and ' mca_caps = [ "lmce" ] ' 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: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Changes in v2: * Allow restore an LMCE enabled guest on an LMCE-incapable host. * Change xl config to "mca_caps = [ "lmce" ]", which can be extended to support other MCA capabilities in the future. --- docs/man/xl.cfg.pod.5.in | 24 ++++++++++++++++++++++++ tools/libxl/libxl_dom.c | 30 ++++++++++++++++++++++++++++++ tools/libxl/libxl_types.idl | 1 + tools/xl/xl_parse.c | 4 ++++ xen/arch/x86/cpu/mcheck/vmce.c | 19 ++++++++++++++++++- xen/arch/x86/hvm/hvm.c | 5 +++++ xen/include/asm-x86/mce.h | 1 + xen/include/public/hvm/params.h | 7 ++++++- 8 files changed, 89 insertions(+), 2 deletions(-)