From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kai Huang Subject: Re: [PATCH] x86/MCE: bypass uninitialized vcpu in vMCE injection Date: Wed, 07 May 2014 16:37:58 +0800 Message-ID: <5369F0E6.5000509@linux.intel.com> References: <1399447758-11798-1-git-send-email-kai.huang@linux.intel.com> <536A09B3020000780000FB5A@mail.emea.novell.com> <5369EE86.7080900@linux.intel.com> <536A0D22020000780000FBF6@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <536A0D22020000780000FBF6@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: yang.z.zhang@intel.com, jinsong.liu@alibaba-inc.com, dongxiao.xu@intel.com, kevin.tian@intel.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 05/07/2014 04:38 PM, Jan Beulich wrote: >>>> On 07.05.14 at 10:27, wrote: >> On 05/07/2014 04:23 PM, Jan Beulich wrote: >>>>>> On 07.05.14 at 09:29, 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 >