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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.02.17 at 06:14, <haozhong.zhang@xxxxxxxxx> wrote:
> On 02/22/17 08:59 -0700, Jan Beulich wrote:
>> >>> On 17.02.17 at 07:39, <haozhong.zhang@xxxxxxxxx> wrote:
>> > @@ -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.

To be honest, I don't see any such check. But looking at that code
makes me notice you should also forbid the combination of
XEN_MC_INJECT_CPU_BROADCAST and XEN_MC_INJECT_TYPE_LMCE.

>> > +            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.

And another one before the new case label you add.

Jan


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