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;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [patch 13/16] x86/entry: Move irqflags and context tracking to C for simple idtentries
Date:   Wed, 26 Feb 2020 09:09:34 -0800
Message-Id: <2A899107-5AB8-4907-8AF2-31154A2E0A98@amacapital.net>
References: <20200226170534.GA6075@lenoir>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>, x86@kernel.org,
        Steven Rostedt <rostedt@goodmis.org>,
        Brian Gerst <brgerst@gmail.com>,
        Juergen Gross <JGross@suse.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Arnd Bergmann <arnd@arndb.de>
In-Reply-To: <20200226170534.GA6075@lenoir>
To:     Frederic Weisbecker <frederic@kernel.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Feb 26, 2020, at 9:05 AM, Frederic Weisbecker <frederic@kernel.org> wro=
te:
>=20
> =EF=BB=BFOn Wed, Feb 26, 2020 at 09:05:38AM +0100, Peter Zijlstra wrote:
>>> On Tue, Feb 25, 2020 at 11:33:34PM +0100, Thomas Gleixner wrote:
>>>=20
>>> --- a/arch/x86/include/asm/idtentry.h
>>> +++ b/arch/x86/include/asm/idtentry.h
>>> @@ -7,14 +7,31 @@
>>>=20
>>> #ifndef __ASSEMBLY__
>>>=20
>>> +#ifdef CONFIG_CONTEXT_TRACKING
>>> +static __always_inline void enter_from_user_context(void)
>>> +{
>>> +    CT_WARN_ON(ct_state() !=3D CONTEXT_USER);
>>> +    user_exit_irqoff();
>>> +}
>>> +#else
>>> +static __always_inline void enter_from_user_context(void) { }
>>> +#endif
>>> +
>>> /**
>>>  * idtentry_enter - Handle state tracking on idtentry
>>>  * @regs:    Pointer to pt_regs of interrupted context
>>>  *
>>> - * Place holder for now.
>>> + * Invokes:
>>> + *  - The hardirq tracer to keep the state consistent as low level ASM
>>> + *    entry disabled interrupts.
>>> + *
>>> + *  - Context tracking if the exception hit user mode
>>>  */
>>> static __always_inline void idtentry_enter(struct pt_regs *regs)
>>> {
>>> +    trace_hardirqs_off();
>>> +    if (user_mode(regs))
>>> +        enter_from_user_context();
>>> }
>>=20
>> So:
>>=20
>>    asm_exc_int3
>>      exc_int3
>>        idtentry_enter()
>>          enter_from_user_context
>>            if (context_tracking_enabled())
>>=20
>>        poke_int3_handler();
>>=20
>> Is, AFAICT, completely buggered.
>>=20
>> You can't have a static_branch before the poke_int3_handler that deals
>> with text_poke.
>=20
> #BP is treated like an NMI in your patchset IIRC?
> In that case and since that implies we can't schedule, we can remove
> the call to context tracking there once we have nmi_enter()/nmi_exit()
> called unconditionally.

int3 from user mode can send signals. This has better be able to schedule by=
 the time it hits prepare_exit_to_usermode.=