From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] x86/hvm: Fix the unknown nested vmexit reason 80000021 bug Date: Mon, 13 Apr 2015 16:17:20 +0100 Message-ID: <552BFA2002000078000718F0@mail.emea.novell.com> References: <1428413222-12120-1-git-send-email-liang.z.li@intel.com> <552401B7.8080701@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap , liang.z.li@intel.com Cc: Andrew Cooper , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org >>> On 08.04.15 at 11:20, wrote: > On Tue, Apr 7, 2015 at 5:11 PM, Andrew Cooper > wrote: >> On 07/04/15 14:27, Liang Li wrote: >>> This bug will be trigged when NMI happen in the L2 guest. The current >>> code handles the NMI incorrectly. According to Intel SDM 31.7.1.2 >>> (Resuming Guest Software after Handling an Exception), If bit 31 of the >>> IDT-vectoring information fields is set, and the virtual NMIs VM-execution >>> control is 1, while bits 10:8 in the IDT-vectoring information field is >>> 2, bit 3 in the interruptibility-state field should be cleared to avoid >>> the next VM entry fail. >>> >>> Signed-off-by: Liang Li >>> --- >>> xen/arch/x86/hvm/vmx/vmx.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >>> index e1c55ce..b1f2df8 100644 >>> --- a/xen/arch/x86/hvm/vmx/vmx.c >>> +++ b/xen/arch/x86/hvm/vmx/vmx.c >>> @@ -2621,7 +2621,8 @@ static void vmx_idtv_reinject(unsigned long idtv_info) >>> * Clear NMI-blocking interruptibility info if an NMI delivery > faulted. >>> * Re-delivery will re-set it (see SDM 3B 25.7.1.2). >>> */ >>> - if ( (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8) ) >>> + if ( cpu_has_vmx_vnmi && ((idtv_info & INTR_INFO_INTR_TYPE_MASK) == >>> + (X86_EVENTTYPE_NMI<<8)) ) >> >> This would be easier to read as >> >> if ( cpu_has_vmx_vnmi && >> (idtv_info & INTR_INFO_INTR_TYPE_MASK) == (X86_EVENTTYPE_NMI<<8)) ) > > I was going to say something similar, but I think in the past Jan has > said that Liang's original is more in line with the coding style. No, my complaint here wouldn't be about coding style, but about the hard-coded 8 - it's not been that long ago that I replaced may of them, and I'd really like to see it replaced here too. Liang - can you please submit an incremental change (as the original one got committed already)? There should be several examples in VMX code on how the 8 can be avoided. Jan