From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751501AbcATFnL (ORCPT ); Wed, 20 Jan 2016 00:43:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49688 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751099AbcATFnB (ORCPT ); Wed, 20 Jan 2016 00:43:01 -0500 Date: Tue, 19 Jan 2016 23:42:56 -0600 From: Josh Poimboeuf To: Ingo Molnar Cc: Borislav Petkov , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, Michal Marek , Peter Zijlstra , Andy Lutomirski , Linus Torvalds , Andi Kleen , Pedro Alves , Namhyung Kim , Bernd Petrovitsch , Chris J Arges , Andrew Morton , Jiri Slaby , Arnaldo Carvalho de Melo Subject: Re: [PATCH v15 13/25] x86/reboot: Add ljmp instructions to stacktool whitelist Message-ID: <20160120054256.GA31495@treble.redhat.com> References: <20160112164711.GD22699@pd.tnic> <20160112174301.GD310@treble.redhat.com> <20160113105503.GB11575@gmail.com> <20160115060652.GA16760@treble.redhat.com> <20160115104145.GC25104@pd.tnic> <20160115110000.GB25002@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20160115110000.GB25002@gmail.com> User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 15, 2016 at 12:00:00PM +0100, Ingo Molnar wrote: > > * Borislav Petkov wrote: > > > On Fri, Jan 15, 2016 at 12:06:52AM -0600, Josh Poimboeuf wrote: > > > - xen_cpuid() uses some custom xen instructions which start with > > > XEN_EMULATE_PREFIX. It corresponds to the following x86 instructions: > > > > > > ffffffff8107e572: 0f 0b ud2 > > > ffffffff8107e574: 78 65 js ffffffff8107e5db > > > ffffffff8107e576: 6e outsb %ds:(%rsi),(%dx) > > > > > > Apparently(?) xen treats the ud2 special when it's followed by "78 65 > > > 6e". This is confusing for stacktool because ud2 is normally a dead > > > end, and it thinks the instructions after it will never run. > > > > > > (In theory stacktool could be taught to understand this hack, but > > > that's a bad idea IMO) > > > > Why, because it is not generic enough? > > > > Well, you could add a cmdline option "--kernel" which is supplied when > > checking the kernel and such kernel "idiosyncrasies" are handled only > > then and there. And since the tool is part of the kernel, changes to > > XEN_EMULATE_PREFIX, will have to be updated in stacktool too... > > So I think because we are talking about less than a dozen annotations, these are > technicalities - and it might in fact be better to have a single line of obvious > annotation in a function that does something weird (and arguably all of these > functions do something weird), than having dozens of lines of code on the tooling > side to avoid that single line on the kernel side. > > That has a documentation value as well. > > As long as the annotation itself is not stacktool specific, it should serve as > documentation as well - such as: > > __non_standard_stack_frame > > or: > > __non_C_instructions > > ? > > All of the cases Josh listed involve some sort of special case where we do > something non-standard. (Where 'standard' == 'regular kernel C function'.) I've now gotten the number of warnings down to 0 (except for a few staging drivers), even with allyesconfig (with !CONFIG_GCOV). I've also managed to make stacktool a little smarter such that the in-code STACKTOOL_IGNORE_INSN markers are no longer needed, woot! There's still a need for 4 STACKTOOL_IGNORE_FUNC(name) markers in the entire tree, due to the weird cases I mentioned. But they're placed after the functions, so they're much less disruptive. I'll be posting a v16 soon. -- Josh