From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752342AbdK0Nbu (ORCPT ); Mon, 27 Nov 2017 08:31:50 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:25595 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752227AbdK0Nbr (ORCPT ); Mon, 27 Nov 2017 08:31:47 -0500 Subject: Re: Xen PV breakage after IRQ stack code refactoring To: Juergen Gross , Andy Lutomirski References: <2a0c08b6-9568-7542-f5db-0eb6c828e39f@suse.com> Cc: xen-devel , "linux-kernel@vger.kernel.org" From: Boris Ostrovsky Message-ID: Date: Mon, 27 Nov 2017 08:30:51 -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: <2a0c08b6-9568-7542-f5db-0eb6c828e39f@suse.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/27/2017 12:34 AM, Juergen Gross wrote: > On 27/11/17 05:03, Andy Lutomirski wrote: >> On Sun, Nov 26, 2017 at 9:10 AM, Boris Ostrovsky >> wrote: >>> Andy, >>> >>> (Can't find the original patch in my mailbox) >>> >>> This hunk from 1d3e53e8624a ("x86/entry/64: Refactor IRQ stacks and make >>> them NMI-safe") >>> >>> >>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S >>> index a9a8027..0d4483a 100644 >>> --- a/arch/x86/entry/entry_64.S >>> +++ b/arch/x86/entry/entry_64.S >>> @@ -447,6 +447,59 @@ ENTRY(irq_entries_start) >>> .endr >>> END(irq_entries_start) >>> >>> +.macro DEBUG_ENTRY_ASSERT_IRQS_OFF >>> +#ifdef CONFIG_DEBUG_ENTRY >>> + pushfq >>> + testl $X86_EFLAGS_IF, (%rsp) >>> + jz .Lokay_\@ >>> + ud2 >>> +.Lokay_\@: >>> + addq $8, %rsp >>> +#endif >>> +.endm >>> + >>> >>> makes Xen PV guests somewhat unhappy because IF flag will be set. >>> >>> I was hoping to use ALTERNATIVE instruction but when we hit this for the >>> first time we haven't rewritten instructions yet. Moving check_bugs() a bit >>> higher helps but because this is common code I don't know how well it will >>> work on other architectures (and, in fact, whether it is even safe on x86 in >>> general, although that can be verified). >>> >>> Another option is to also add a parameter to DEBUG_ENTRY_ASSERT_IRQS_OFF (or >>> to ENTER_IRQ_STACK) from xen_do_hypervisor_callback (which is where the >>> failure happens) but this looks pretty fragile in that it assumes that >>> xen_do_hypervisor_callback is the only place where we use this codepath >>> before alt instructions are set. >>> >>> Any other suggestions? >> Do we have a convenient asm way to access the save_fl pvop? > No, but adding it would be pretty straight forward. Its something like: > > #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);) > > similar to DISABLE_INTERRUPTS(), requiring just the definition of > PV_IRQ_save_fl in asm-offsets.c and the non-pvops definition of > SAVE_FLAGS() in irqflags.h. Hmm.. Indeed, I haven't thought of this for whatever reasons. I'll send the patch soon. Thanks. -boris