Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751169AbdLAQWZ (ORCPT ); Fri, 1 Dec 2017 11:22:25 -0500 Received: from mail.kernel.org ([198.145.29.99]:60572 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750742AbdLAQWX (ORCPT ); Fri, 1 Dec 2017 11:22:23 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 52F1B2187C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=luto@kernel.org X-Google-Smtp-Source: AGs4zMZTa9Cg79ghMjU23VLi0FrFchNsCusDUHWEoyO86QMbijxFU+utD1TVyMaCOdav9PjHh6XMTQy2V9eUVQ7mxXk= MIME-Version: 1.0 In-Reply-To: <1511882905-6326-1-git-send-email-boris.ostrovsky@oracle.com> References: <1511882905-6326-1-git-send-email-boris.ostrovsky@oracle.com> From: Andy Lutomirski Date: Fri, 1 Dec 2017 08:22:02 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2] x86/entry/64/paravirt: Use paravirt-safe macro to access eflags To: Boris Ostrovsky Cc: "xen-devel@lists.xenproject.org" , "linux-kernel@vger.kernel.org" , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , X86 ML , Juergen Gross , Andrew Lutomirski Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3594 Lines: 98 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? > jz .Lokay_\@ > ud2 > .Lokay_\@: > - addq $8, %rsp > + popq %rax > #endif > .endm > > diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h > index c8ef23f..89f0895 100644 > --- a/arch/x86/include/asm/irqflags.h > +++ b/arch/x86/include/asm/irqflags.h > @@ -142,6 +142,9 @@ static inline notrace unsigned long arch_local_irq_save(void) > swapgs; \ > sysretl > > +#ifdef CONFIG_DEBUG_ENTRY > +#define SAVE_FLAGS(x) pushfq; popq %rax > +#endif > #else > #define INTERRUPT_RETURN iret > #define ENABLE_INTERRUPTS_SYSEXIT sti; sysexit > diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h > index 283efca..892df37 100644 > --- a/arch/x86/include/asm/paravirt.h > +++ b/arch/x86/include/asm/paravirt.h > @@ -927,6 +927,15 @@ static inline notrace unsigned long arch_local_irq_save(void) > PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_usergs_sysret64), \ > CLBR_NONE, \ > jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret64)) > + > +#ifdef CONFIG_DEBUG_ENTRY > +#define SAVE_FLAGS(clobbers) \ > + PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_save_fl), clobbers, \ > + PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE); \ > + call PARA_INDIRECT(pv_irq_ops+PV_IRQ_save_fl); \ > + PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);) > +#endif > + > #endif /* CONFIG_X86_32 */ > > #endif /* __ASSEMBLY__ */ > diff --git a/arch/x86/kernel/asm-offsets_64.c b/arch/x86/kernel/asm-offsets_64.c > index 630212f..e3a5175 100644 > --- a/arch/x86/kernel/asm-offsets_64.c > +++ b/arch/x86/kernel/asm-offsets_64.c > @@ -23,6 +23,9 @@ int main(void) > #ifdef CONFIG_PARAVIRT > OFFSET(PV_CPU_usergs_sysret64, pv_cpu_ops, usergs_sysret64); > OFFSET(PV_CPU_swapgs, pv_cpu_ops, swapgs); > +#ifdef CONFIG_DEBUG_ENTRY > + OFFSET(PV_IRQ_save_fl, pv_irq_ops, save_fl); > +#endif > BLANK(); > #endif > > -- > 1.7.1 >