| Message ID | 20170217063936.13208-20-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 17, 2017 at 02:39:36PM +0800, Haozhong Zhang wrote: > If option '-l' or '--lmce' is specified and the host supports LMCE, > xen-mceinj will inject LMCE to CPU specified by '-c' (or CPU0 if '-c' > is not present). > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > --- > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > --- > tools/libxc/include/xenctrl.h | 1 + > tools/libxc/xc_misc.c | 25 +++++++++++++++ I suggest you split out changes to libxc to a separate patch. > tools/tests/mce-test/tools/xen-mceinj.c | 57 +++++++++++++++++++++++++++++++-- > 3 files changed, 81 insertions(+), 2 deletions(-) > > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > index 85d7fe5..2598952 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -1968,6 +1968,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, > void xc_cpuid_to_str(const unsigned int *regs, > char **strs); /* some strs[] may be NULL if ENOMEM */ > int xc_mca_op(xc_interface *xch, struct xen_mc *mc); > +int xc_mca_op_cpumap(xc_interface *xch, struct xen_mc *mc, xc_cpumap_t cpumap); > #endif > > struct xc_px_val { > diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c > index 0fc6c22..24f7fdf 100644 > --- a/tools/libxc/xc_misc.c > +++ b/tools/libxc/xc_misc.c > @@ -341,6 +341,31 @@ int xc_mca_op(xc_interface *xch, struct xen_mc *mc) > xc_hypercall_bounce_post(xch, mc); > return ret; > } > + > +int xc_mca_op_cpumap(xc_interface *xch, struct xen_mc *mc, xc_cpumap_t cpumap) > +{ > + int ret; > + DECLARE_HYPERCALL_BOUNCE(cpumap, 0, XC_HYPERCALL_BUFFER_BOUNCE_IN); > + > + if ( cpumap ) > + { > + HYPERCALL_BOUNCE_SET_SIZE(cpumap, > + (mc->u.mc_inject_v2.cpumap.nr_bits + 7) / 8); > + if ( xc_hypercall_bounce_pre(xch, cpumap) ) > + { > + PERROR("Could not bouce cpumap memory buffer"); > + return -1; > + } > + set_xen_guest_handle(mc->u.mc_inject_v2.cpumap.bitmap, cpumap); > + } > + > + ret = xc_mca_op(xch, mc); > + > + if ( cpumap ) > + xc_hypercall_bounce_post(xch, cpumap); > + > + return ret; > +} I kinda see why you did this: the bounce buffer infrastructure isn't available to userspace program (by design). But this API isn't nice. This function replaces part of the struct. I suggest you construct a xen_mc struct solely within this function, not doing part of it here and the other part in another place (inject_lmce below). Then you also need to name it properly. > #endif > > int xc_perfc_reset(xc_interface *xch) > diff --git a/tools/tests/mce-test/tools/xen-mceinj.c b/tools/tests/mce-test/tools/xen-mceinj.c > index 5f70a61..b2eb7d3 100644 > --- a/tools/tests/mce-test/tools/xen-mceinj.c > +++ b/tools/tests/mce-test/tools/xen-mceinj.c > @@ -56,6 +56,8 @@ > #define MSR_IA32_MC0_MISC 0x00000403 > #define MSR_IA32_MC0_CTL2 0x00000280 > > +#define MCG_STATUS_LMCE 0x8 > + > struct mce_info { > const char *description; > uint8_t mcg_stat; > @@ -113,6 +115,7 @@ static struct mce_info mce_table[] = { > #define LOGFILE stdout > > int dump; > +int lmce; > struct xen_mc_msrinject msr_inj; > > static void Lprintf(const char *fmt, ...) > @@ -213,6 +216,42 @@ static int inject_mce(xc_interface *xc_handle, int cpu_nr) > return xc_mca_op(xc_handle, &mc); > } > > +static int inject_lmce(xc_interface *xc_handle, uint32_t cpu_nr) > +{ > + struct xen_mc mc; > + uint8_t *cpumap = NULL; > + size_t cpumap_size, line, shift; > + uint32_t nr_cpus; > + int ret; > + > + nr_cpus = mca_cpuinfo(xc_handle); > + if ( !nr_cpus ) > + err(xc_handle, "Failed to get mca_cpuinfo"); Why xc_handle in err()? Wei.
On 02/20/17 12:53 +0000, Wei Liu wrote: > On Fri, Feb 17, 2017 at 02:39:36PM +0800, Haozhong Zhang wrote: > > If option '-l' or '--lmce' is specified and the host supports LMCE, > > xen-mceinj will inject LMCE to CPU specified by '-c' (or CPU0 if '-c' > > is not present). > > > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > --- > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > Cc: Wei Liu <wei.liu2@citrix.com> > > --- > > tools/libxc/include/xenctrl.h | 1 + > > tools/libxc/xc_misc.c | 25 +++++++++++++++ > > I suggest you split out changes to libxc to a separate patch. > will do in the next version > > tools/tests/mce-test/tools/xen-mceinj.c | 57 +++++++++++++++++++++++++++++++-- > > 3 files changed, 81 insertions(+), 2 deletions(-) > > > > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > > index 85d7fe5..2598952 100644 > > --- a/tools/libxc/include/xenctrl.h > > +++ b/tools/libxc/include/xenctrl.h > > @@ -1968,6 +1968,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, > > void xc_cpuid_to_str(const unsigned int *regs, > > char **strs); /* some strs[] may be NULL if ENOMEM */ > > int xc_mca_op(xc_interface *xch, struct xen_mc *mc); > > +int xc_mca_op_cpumap(xc_interface *xch, struct xen_mc *mc, xc_cpumap_t cpumap); > > #endif > > > > struct xc_px_val { > > diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c > > index 0fc6c22..24f7fdf 100644 > > --- a/tools/libxc/xc_misc.c > > +++ b/tools/libxc/xc_misc.c > > @@ -341,6 +341,31 @@ int xc_mca_op(xc_interface *xch, struct xen_mc *mc) > > xc_hypercall_bounce_post(xch, mc); > > return ret; > > } > > + > > +int xc_mca_op_cpumap(xc_interface *xch, struct xen_mc *mc, xc_cpumap_t cpumap) > > +{ > > + int ret; > > + DECLARE_HYPERCALL_BOUNCE(cpumap, 0, XC_HYPERCALL_BUFFER_BOUNCE_IN); > > + > > + if ( cpumap ) > > + { > > + HYPERCALL_BOUNCE_SET_SIZE(cpumap, > > + (mc->u.mc_inject_v2.cpumap.nr_bits + 7) / 8); > > + if ( xc_hypercall_bounce_pre(xch, cpumap) ) > > + { > > + PERROR("Could not bouce cpumap memory buffer"); > > + return -1; > > + } > > + set_xen_guest_handle(mc->u.mc_inject_v2.cpumap.bitmap, cpumap); > > + } > > + > > + ret = xc_mca_op(xch, mc); > > + > > + if ( cpumap ) > > + xc_hypercall_bounce_post(xch, cpumap); > > + > > + return ret; > > +} > > I kinda see why you did this: the bounce buffer infrastructure isn't > available to userspace program (by design). But this API isn't nice. > > This function replaces part of the struct. I suggest you construct a > xen_mc struct solely within this function, not doing part of it here and > the other part in another place (inject_lmce below). Then you also need > to name it properly. > ditto > > #endif > > > > int xc_perfc_reset(xc_interface *xch) > > diff --git a/tools/tests/mce-test/tools/xen-mceinj.c b/tools/tests/mce-test/tools/xen-mceinj.c > > index 5f70a61..b2eb7d3 100644 > > --- a/tools/tests/mce-test/tools/xen-mceinj.c > > +++ b/tools/tests/mce-test/tools/xen-mceinj.c > > @@ -56,6 +56,8 @@ > > #define MSR_IA32_MC0_MISC 0x00000403 > > #define MSR_IA32_MC0_CTL2 0x00000280 > > > > +#define MCG_STATUS_LMCE 0x8 > > + > > struct mce_info { > > const char *description; > > uint8_t mcg_stat; > > @@ -113,6 +115,7 @@ static struct mce_info mce_table[] = { > > #define LOGFILE stdout > > > > int dump; > > +int lmce; > > struct xen_mc_msrinject msr_inj; > > > > static void Lprintf(const char *fmt, ...) > > @@ -213,6 +216,42 @@ static int inject_mce(xc_interface *xc_handle, int cpu_nr) > > return xc_mca_op(xc_handle, &mc); > > } > > > > +static int inject_lmce(xc_interface *xc_handle, uint32_t cpu_nr) > > +{ > > + struct xen_mc mc; > > + uint8_t *cpumap = NULL; > > + size_t cpumap_size, line, shift; > > + uint32_t nr_cpus; > > + int ret; > > + > > + nr_cpus = mca_cpuinfo(xc_handle); > > + if ( !nr_cpus ) > > + err(xc_handle, "Failed to get mca_cpuinfo"); > > Why xc_handle in err()? > err() needs to close xc_handle. Thanks, Haozhong
On Tue, Feb 21, 2017 at 07:50:38AM +0800, Haozhong Zhang wrote: > On 02/20/17 12:53 +0000, Wei Liu wrote: > > > } > > > > > > +static int inject_lmce(xc_interface *xc_handle, uint32_t cpu_nr) > > > +{ > > > + struct xen_mc mc; > > > + uint8_t *cpumap = NULL; > > > + size_t cpumap_size, line, shift; > > > + uint32_t nr_cpus; > > > + int ret; > > > + > > > + nr_cpus = mca_cpuinfo(xc_handle); > > > + if ( !nr_cpus ) > > > + err(xc_handle, "Failed to get mca_cpuinfo"); > > > > Why xc_handle in err()? > > > > err() needs to close xc_handle. > Gosh, this is err in xen-mceinj.c, not err(3). Sorry for the noise. Wei.
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 85d7fe5..2598952 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -1968,6 +1968,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, void xc_cpuid_to_str(const unsigned int *regs, char **strs); /* some strs[] may be NULL if ENOMEM */ int xc_mca_op(xc_interface *xch, struct xen_mc *mc); +int xc_mca_op_cpumap(xc_interface *xch, struct xen_mc *mc, xc_cpumap_t cpumap); #endif struct xc_px_val { diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c index 0fc6c22..24f7fdf 100644 --- a/tools/libxc/xc_misc.c +++ b/tools/libxc/xc_misc.c @@ -341,6 +341,31 @@ int xc_mca_op(xc_interface *xch, struct xen_mc *mc) xc_hypercall_bounce_post(xch, mc); return ret; } + +int xc_mca_op_cpumap(xc_interface *xch, struct xen_mc *mc, xc_cpumap_t cpumap) +{ + int ret; + DECLARE_HYPERCALL_BOUNCE(cpumap, 0, XC_HYPERCALL_BUFFER_BOUNCE_IN); + + if ( cpumap ) + { + HYPERCALL_BOUNCE_SET_SIZE(cpumap, + (mc->u.mc_inject_v2.cpumap.nr_bits + 7) / 8); + if ( xc_hypercall_bounce_pre(xch, cpumap) ) + { + PERROR("Could not bouce cpumap memory buffer"); + return -1; + } + set_xen_guest_handle(mc->u.mc_inject_v2.cpumap.bitmap, cpumap); + } + + ret = xc_mca_op(xch, mc); + + if ( cpumap ) + xc_hypercall_bounce_post(xch, cpumap); + + return ret; +} #endif int xc_perfc_reset(xc_interface *xch) diff --git a/tools/tests/mce-test/tools/xen-mceinj.c b/tools/tests/mce-test/tools/xen-mceinj.c index 5f70a61..b2eb7d3 100644 --- a/tools/tests/mce-test/tools/xen-mceinj.c +++ b/tools/tests/mce-test/tools/xen-mceinj.c @@ -56,6 +56,8 @@ #define MSR_IA32_MC0_MISC 0x00000403 #define MSR_IA32_MC0_CTL2 0x00000280 +#define MCG_STATUS_LMCE 0x8 + struct mce_info { const char *description; uint8_t mcg_stat; @@ -113,6 +115,7 @@ static struct mce_info mce_table[] = { #define LOGFILE stdout int dump; +int lmce; struct xen_mc_msrinject msr_inj; static void Lprintf(const char *fmt, ...) @@ -213,6 +216,42 @@ static int inject_mce(xc_interface *xc_handle, int cpu_nr) return xc_mca_op(xc_handle, &mc); } +static int inject_lmce(xc_interface *xc_handle, uint32_t cpu_nr) +{ + struct xen_mc mc; + uint8_t *cpumap = NULL; + size_t cpumap_size, line, shift; + uint32_t nr_cpus; + int ret; + + nr_cpus = mca_cpuinfo(xc_handle); + if ( !nr_cpus ) + err(xc_handle, "Failed to get mca_cpuinfo"); + if ( cpu_nr >= nr_cpus ) + err(xc_handle, "-c %"PRIu32" is larger than %"PRIu32, + cpu_nr, nr_cpus - 1); + + memset(&mc, 0, sizeof(struct xen_mc)); + mc.cmd = XEN_MC_inject_v2; + mc.interface_version = XEN_MCA_INTERFACE_VERSION; + mc.u.mc_inject_v2.flags |= XEN_MC_INJECT_TYPE_LMCE; + + cpumap_size = (nr_cpus + 7) / 8; + cpumap = malloc(cpumap_size); + if ( !cpumap ) + err(xc_handle, "Failed to allocate cpumap\n"); + memset(cpumap, 0, cpumap_size); + line = cpu_nr / 8; + shift = cpu_nr % 8; + memset(cpumap + line, 1 << shift, 1); + + mc.u.mc_inject_v2.cpumap.nr_bits = cpumap_size * 8; + ret = xc_mca_op_cpumap(xc_handle, &mc, cpumap); + + free(cpumap); + return ret; +} + static uint64_t bank_addr(int bank, int type) { uint64_t addr; @@ -331,8 +370,15 @@ static int inject(xc_interface *xc_handle, struct mce_info *mce, uint32_t cpu_nr, uint32_t domain, uint64_t gaddr) { int ret = 0; + uint8_t mcg_status = mce->mcg_stat; - ret = inject_mcg_status(xc_handle, cpu_nr, mce->mcg_stat, domain); + if ( lmce ) + { + if ( mce->cmci ) + err(xc_handle, "No support to inject CMCI as LMCE"); + mcg_status |= MCG_STATUS_LMCE; + } + ret = inject_mcg_status(xc_handle, cpu_nr, mcg_status, domain); if ( ret ) err(xc_handle, "Failed to inject MCG_STATUS MSR"); @@ -355,6 +401,8 @@ static int inject(xc_interface *xc_handle, struct mce_info *mce, err(xc_handle, "Failed to inject MSR"); if ( mce->cmci ) ret = inject_cmci(xc_handle, cpu_nr); + else if ( lmce ) + ret = inject_lmce(xc_handle, cpu_nr); else ret = inject_mce(xc_handle, cpu_nr); if ( ret ) @@ -394,6 +442,7 @@ static struct option opts[] = { {"dump", 0, 0, 'D'}, {"help", 0, 0, 'h'}, {"page", 0, 0, 'p'}, + {"lmce", 0, 0, 'l'}, {"", 0, 0, '\0'} }; @@ -410,6 +459,7 @@ static void help(void) " -d, --domain=DOMID target domain, the default is Xen itself\n" " -h, --help print this page\n" " -p, --page=ADDR physical address to report\n" + " -l, --lmce inject as LMCE (Intel only)\n" " -t, --type=ERROR error type\n"); for ( i = 0; i < MCE_TABLE_SIZE; i++ ) @@ -439,7 +489,7 @@ int main(int argc, char *argv[]) } while ( 1 ) { - c = getopt_long(argc, argv, "c:Dd:t:hp:", opts, &opt_index); + c = getopt_long(argc, argv, "c:Dd:t:hp:l", opts, &opt_index); if ( c == -1 ) break; switch ( c ) { @@ -464,6 +514,9 @@ int main(int argc, char *argv[]) case 't': type = strtol(optarg, NULL, 0); break; + case 'l': + lmce = 1; + break; case 'h': default: help();
If option '-l' or '--lmce' is specified and the host supports LMCE, xen-mceinj will inject LMCE to CPU specified by '-c' (or CPU0 if '-c' is not present). Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> --- tools/libxc/include/xenctrl.h | 1 + tools/libxc/xc_misc.c | 25 +++++++++++++++ tools/tests/mce-test/tools/xen-mceinj.c | 57 +++++++++++++++++++++++++++++++-- 3 files changed, 81 insertions(+), 2 deletions(-)