From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751156AbdK0FeE (ORCPT ); Mon, 27 Nov 2017 00:34:04 -0500 Received: from mx2.suse.de ([195.135.220.15]:50267 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750743AbdK0FeD (ORCPT ); Mon, 27 Nov 2017 00:34:03 -0500 Subject: Re: Xen PV breakage after IRQ stack code refactoring To: Andy Lutomirski , Boris Ostrovsky Cc: xen-devel , "linux-kernel@vger.kernel.org" References: From: Juergen Gross Message-ID: <2a0c08b6-9568-7542-f5db-0eb6c828e39f@suse.com> Date: Mon, 27 Nov 2017 06:34:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. Juergen