Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: Xen PV breakage after IRQ stack code refactoring
To:     Juergen Gross <jgross@suse.com>, Andy Lutomirski <luto@kernel.org>
References: <daa7c310-ca76-0f72-bd50-2d188abc4364@oracle.com>
 <CALCETrWj5qcPbG4JU1koL+oh6Sv43=BaHcFsb1S-qazFesnxDA@mail.gmail.com>
 <2a0c08b6-9568-7542-f5db-0eb6c828e39f@suse.com>
Cc:     xen-devel <xen-devel@lists.xen.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
From:   Boris Ostrovsky <boris.ostrovsky@oracle.com>
Message-ID: <ea6f3707-afcf-beda-2a91-fa40a9698438@oracle.com>
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
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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
>> <boris.ostrovsky@oracle.com> 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

From 1585196376668958117@xxx Mon Nov 27 05:37:50 +0000 2017
X-GM-THRID: 1585149410444720891
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread