From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932549AbcLHPsQ (ORCPT ); Thu, 8 Dec 2016 10:48:16 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:53262 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753471AbcLHPre (ORCPT ); Thu, 8 Dec 2016 10:47:34 -0500 Message-Id: <20161208154349.346057680@infradead.org> User-Agent: quilt/0.63-1 Date: Thu, 08 Dec 2016 16:42:15 +0100 From: Peter Zijlstra To: linux-kernel@vger.kernel.org, x86@kernel.org Cc: Peter Anvin , Ingo Molnar , Thomas Gleixner , Borislav Petkov , Jeremy Fitzhardinge , Chris Wright , Alok Kataria , Rusty Russell , virtualization@lists.linux-foundation.org, Pan Xinhui , Paolo Bonzini , kernel test robot , "Peter Zijlstra (Intel)" Subject: [PATCH 2/2] x86, paravirt: Fix bool return type for PVOP_CALL References: <20161208154213.952687487@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline; filename=peterz-fix-paravirt-bool.patch Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit 3cded4179481 ("x86/paravirt: Optimize native pv_lock_ops.vcpu_is_preempted()") introduced a paravirt op with bool return type [*] It turns out that the PVOP_CALL*() macros miscompile when rettype is bool. Code that looked like: 83 ef 01 sub $0x1,%edi ff 15 32 a0 d8 00 callq *0xd8a032(%rip) # ffffffff81e28120 84 c0 test %al,%al ended up looking like so after PVOP_CALL1() was applied: 83 ef 01 sub $0x1,%edi 48 63 ff movslq %edi,%rdi ff 14 25 20 81 e2 81 callq *0xffffffff81e28120 48 85 c0 test %rax,%rax Note how it tests the whole of %rax, even though a typical bool return function only sets %al, like: 0f 95 c0 setne %al c3 retq This is because ____PVOP_CALL() does: __ret = (rettype)__eax; and while regular integer type casts truncate the result, a cast to bool tests for any !0 value. Fix this by explicitly truncating to sizeof(rettype) before casting. [*] The actual bug should've been exposed in commit 446f3dc8cc0a ("locking/core, x86/paravirt: Implement vcpu_is_preempted(cpu) for KVM and Xen guests") but that didn't properly implement the paravirt call. Cc: Peter Anvin Cc: Ingo Molnar Cc: Thomas Gleixner Cc: Borislav Petkov Cc: Jeremy Fitzhardinge Cc: Chris Wright Cc: Alok Kataria Cc: Rusty Russell Cc: virtualization@lists.linux-foundation.org Cc: Pan Xinhui Cc: Paolo Bonzini Fixes: 3cded4179481 ("x86/paravirt: Optimize native pv_lock_ops.vcpu_is_preempted()") Reported-by: kernel test robot Signed-off-by: Peter Zijlstra (Intel) --- arch/x86/include/asm/paravirt_types.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) --- a/arch/x86/include/asm/paravirt_types.h +++ b/arch/x86/include/asm/paravirt_types.h @@ -508,6 +508,18 @@ int paravirt_disable_iospace(void); #define PVOP_TEST_NULL(op) ((void)op) #endif +#define PVOP_RETMASK(rettype) \ + ({ unsigned long __mask = ~0UL; \ + switch (sizeof(rettype)) { \ + case 1: __mask = 0xffUL; break; \ + case 2: __mask = 0xffffUL; break; \ + case 4: __mask = 0xffffffffUL; break; \ + default: break; \ + } \ + __mask; \ + }) + + #define ____PVOP_CALL(rettype, op, clbr, call_clbr, extra_clbr, \ pre, post, ...) \ ({ \ @@ -535,7 +547,7 @@ int paravirt_disable_iospace(void); paravirt_clobber(clbr), \ ##__VA_ARGS__ \ : "memory", "cc" extra_clbr); \ - __ret = (rettype)__eax; \ + __ret = (rettype)(__eax & PVOP_RETMASK(rettype)); \ } \ __ret; \ })