Re: [PATCH] x86/MCE: bypass uninitialized vcpu in vMCE injection

[Kai Huang <kai.huang@linux.intel.com>]

On 05/07/2014 04:38 PM, Jan Beulich wrote:
>>>> On 07.05.14 at 10:27, <kai.huang@linux.intel.com> wrote:
>> On 05/07/2014 04:23 PM, Jan Beulich wrote:
>>>>>> On 07.05.14 at 09:29, <kai.huang@linux.intel.com> wrote:
>>>> --- a/xen/arch/x86/cpu/mcheck/vmce.c
>>>> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
>>>> @@ -356,6 +356,10 @@ int inject_vmce(struct domain *d, int vcpu)
>>>>            if ( vcpu != VMCE_INJECT_BROADCAST && vcpu != v->vcpu_id )
>>>>                continue;
>>>>    
>>>> +        /* In case of broadcasting, don't inject to uninitialized VCPU */
>>>> +        if ( vcpu == VMCE_INJECT_BROADCAST && !v->is_initialised )
>>>> +            continue;
>>>> +
>>> Conceptually fine, but mechanically in need of improvement: Please
>>> fold the check with the previous one, to avoid checking for
>>> VMCE_INJECT_BROADCAST twice.
>>>
>>> I'd do this as
>>>
>>>           if ( vcpu != VMCE_INJECT_BROADCAST ? vcpu != v->vcpu_id
>>>                                              : !v->is_initialised )
>>>               continue;
>> Thanks for comments. In this case, is it OK to you to add below comments
>> just before the if statement? I think it's better to keep the comments
>> somewhere.
>>
>> 	/* In case of broadcasting, don't inject to uninitialized VCPU */
>> 	if ( vcpu != VMCE_INJECT_BROADCAST ? vcpu != v->vcpu_id
>>                                              : !v->is_initialised )
>>               continue;
>>
> Yes, of course it's fine to keep the comment (but please fix its style
> - it's missing a stop at the end).
Oh, thanks for noticing that. I'll re-send patch after sanity check on 
my testing machine.

Thanks,
-Kai
>
> Jan
>