Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc:     linux-kernel <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>,
        paulmck <paulmck@kernel.org>, Andy Lutomirski <luto@kernel.org>,
        Alexandre Chartre <alexandre.chartre@oracle.com>,
        Frederic Weisbecker <frederic@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <sean.j.christopherson@intel.com>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Petr Mladek <pmladek@suse.com>, rostedt <rostedt@goodmis.org>,
        "Joel Fernandes\, Google" <joel@joelfernandes.org>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Juergen Gross <jgross@suse.com>,
        Brian Gerst <brgerst@gmail.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Will Deacon <will@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling
In-Reply-To: <552488029.20647.1589423096441.JavaMail.zimbra@efficios.com>
References: <20200505134926.578885807@linutronix.de> <20200505135314.808628211@linutronix.de> <552488029.20647.1589423096441.JavaMail.zimbra@efficios.com>
Date:   Thu, 14 May 2020 19:28:32 +0200
Message-ID: <871rnmzarj.fsf@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:
> ----- On May 5, 2020, at 9:49 AM, Thomas Gleixner tglx@linutronix.de wrote:
>> +
>> +static __always_inline void debug_exit(unsigned long dr7)
>> +{
>> +	set_debugreg(dr7, 7);
>> +}
>
> Out of curiosity, what prevents the compiler from moving instructions
> outside of the code regions surrounded by entry/exit ? This is an always
> inline, which invokes set_debugreg which is inline for CONFIG_PARAVIRT_XXL=n,
> which in turn uses an asm() (not volatile), without any memory
> clobber.
>
> Also, considering that "inline" is not sufficient to ensure the compiler
> does not emit a traceable function, I suspect you'll also want to mark
> "native_get_debugreg" and "native_set_debugreg" always inline as well.

It can move it into a function and call that. Fine. If that function
stays in the noinstr section then it should not emit a trace stub and if
it moves it out of the section or reuses another instance in text then
objtool will complain.

Checking for trace stubs and other instrumentation nonsense is on the
objtool wishlist anyway.

But yes, marking these __always_inline prevents that. Those escaped my
chase. But I would have found them once I go and fix that paravirt muck.

Thanks,

        tglx