Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751892AbdLAQ5o (ORCPT ); Fri, 1 Dec 2017 11:57:44 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:29585 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750801AbdLAQ5m (ORCPT ); Fri, 1 Dec 2017 11:57:42 -0500 Subject: Re: [PATCH v2] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags To: Andy Lutomirski References: <1511882905-6326-1-git-send-email-boris.ostrovsky@oracle.com> Cc: "xen-devel@lists.xenproject.org" , "linux-kernel@vger.kernel.org" , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , X86 ML , Juergen Gross From: Boris Ostrovsky Message-ID: Date: Fri, 1 Dec 2017 11:57:21 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1781 Lines: 54 On 12/01/2017 11:22 AM, Andy Lutomirski wrote: > On Tue, Nov 28, 2017 at 7:28 AM, Boris Ostrovsky > wrote: >> Commit 1d3e53e8624a ("x86/entry/64: Refactor IRQ stacks and make >> them NMI-safe") added DEBUG_ENTRY_ASSERT_IRQS_OFF macro that acceses >> eflags using 'pushfq' instruction when testing for IF bit. On PV Xen >> guests looking at IF flag directly will always see it set, resulting >> in 'ud2'. >> >> Introduce SAVE_FLAGS() macro that will use appropriate save_fl pv op >> when running paravirt. >> >> Signed-off-by: Boris Ostrovsky >> --- >> V2: >> * Preserve %rax in DEBUG_ENTRY_ASSERT_IRQS_OFF >> * Return (pop) %rax in SAVE_FLAGS for !CONFIG_PARAVIRT (irqflags.h) >> >> arch/x86/entry/entry_64.S | 7 ++++--- >> arch/x86/include/asm/irqflags.h | 3 +++ >> arch/x86/include/asm/paravirt.h | 9 +++++++++ >> arch/x86/kernel/asm-offsets_64.c | 3 +++ >> 4 files changed, 19 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >> index f81d50d..c208dc1 100644 >> --- a/arch/x86/entry/entry_64.S >> +++ b/arch/x86/entry/entry_64.S >> @@ -466,12 +466,13 @@ END(irq_entries_start) >> >> .macro DEBUG_ENTRY_ASSERT_IRQS_OFF >> #ifdef CONFIG_DEBUG_ENTRY >> - pushfq >> - testl $X86_EFLAGS_IF, (%rsp) >> + pushq %rax >> + SAVE_FLAGS(CLBR_ANY) >> + testl $X86_EFLAGS_IF, %eax > Confused. You're both using CLBR_ANY and RAX. Did you perhaps mean CLBR_NONE? CLBR_NONE will restore all registers, won't it? So it should be CLBR_RAX, should it? Otherwise we'll lose return value. -boris > >> jz .Lokay_\@ >> ud2 >> .Lokay_\@: >> - addq $8, %rsp >> + popq %rax >> #endif >> .endm >> >>