Re: [PATCH 19/19] tools/xen-mceinj: support injecting LMCE

[Wei Liu <wei.liu2@citrix.com>]

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.

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