Re: [Xen-devel] [PATCH 18/19] xen/mce: add support of vLMCE injection to XEN_MC_inject_v2

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On 02/22/17 08:59 -0700, Jan Beulich wrote:
> >>> On 17.02.17 at 07:39, <haozhong.zhang@xxxxxxxxx> wrote:
> > --- a/xen/arch/x86/cpu/mcheck/mce.c
> > +++ b/xen/arch/x86/cpu/mcheck/mce.c
> > @@ -1510,6 +1510,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
> >      {
> >          const cpumask_t *cpumap;
> >          cpumask_var_t cmv;
> > +        int cpu_nr;
> 
> unsigned int (and likely no need for the _nr suffix)
> 
> Or perhaps the local variable isn't needed at all.
>

I'll inline this variable.

> > @@ -1552,6 +1553,21 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) 
> > u_xen_mc)
> >                  send_IPI_mask(cpumap, cmci_apic_vector);
> >              }
> >              break;
> > +        case XEN_MC_INJECT_TYPE_LMCE:
> > +            if ( !lmce_support )
> > +            {
> > +                ret = x86_mcerr("No LMCE support in platform", -EINVAL);
> > +                break;
> > +            }
> > +            /* ensure at most one CPU is specified */
> 
> And what use is none at all? Also - comment style (should start with
> a capital).
>

Do you mean the check of empty cpumap? It's checked at the beginning of case 
XEN_MC_inject_v2.

> > +            cpu_nr = cpumask_next(cpumask_first(cpumap), cpumap);
> > +            if ( cpu_nr < nr_cpu_ids )
> > +            {
> > +                ret = x86_mcerr("More than one CPU specified", -EINVAL);
> > +                break;
> > +            }
> > +            on_selected_cpus(cpumap, x86_mc_mceinject, NULL, 1);
> > +            break;
> >          default:
> 
> See earlier patch comments regarding blank lines missing here.
>

I'll add a blank line before the default case.

Thanks,
Haozhong

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