Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752149AbdLARon (ORCPT ); Fri, 1 Dec 2017 12:44:43 -0500 Received: from mail.kernel.org ([198.145.29.99]:40476 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751085AbdLARom (ORCPT ); Fri, 1 Dec 2017 12:44:42 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DEF02219A4 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: AGs4zMbFcNngUUtm/Bu+1/FIdA1SOfaUMlka2zUa8eJi4fkhroeU8AwTw5XumGw3AEzM6dk2KeIuwNry9v1DYGyuxmU= MIME-Version: 1.0 In-Reply-To: References: <1511882905-6326-1-git-send-email-boris.ostrovsky@oracle.com> From: Andy Lutomirski Date: Fri, 1 Dec 2017 09:44:20 -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: Andy Lutomirski , "xen-devel@lists.xenproject.org" , "linux-kernel@vger.kernel.org" , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , X86 ML , Juergen Gross 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: 1993 Lines: 60 On Fri, Dec 1, 2017 at 8:57 AM, Boris Ostrovsky wrote: > 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. Ugh, probably right. I've never grokked exactly what CLBR_ does. > > -boris > >> >>> jz .Lokay_\@ >>> ud2 >>> .Lokay_\@: >>> - addq $8, %rsp >>> + popq %rax >>> #endif >>> .endm >>> >>> >