From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752583AbdHIIts (ORCPT ); Wed, 9 Aug 2017 04:49:48 -0400 Received: from mx2.suse.de ([195.135.220.15]:56637 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751974AbdHIItr (ORCPT ); Wed, 9 Aug 2017 04:49:47 -0400 Subject: Re: [PATCH v4 1/2] x86/unwind: add ORC unwinder To: Andy Lutomirski , Josh Poimboeuf Cc: Linus Torvalds , "Levin, Alexander (Sasha Levin)" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "live-patching@vger.kernel.org" , Jiri Slaby , Ingo Molnar , "H. Peter Anvin" , Peter Zijlstra , Mike Galbraith References: <0a6cbfb40f8da99b7a45a1a8302dc6aef16ec812.1500938583.git.jpoimboe@redhat.com> <20170728164844.tee7ujqluv2fgarf@sasha-lappy> <20170728175234.pmou7z6dosbi7krh@treble> <20170728182954.imivl45vrc7toarp@sasha-lappy> <20170728185720.fwczcezbkrlxmwqb@treble> <20170728195912.panyn4p6e6ovueey@sasha-lappy> <20170729035437.fmnxmeedrq55bpwm@treble> <20170808185809.qaug762cj7nw3bcd@treble> <20170808191344.kw5nlicmq4g6fpii@treble> From: Juergen Gross Message-ID: <79d6e04a-0a1d-592d-5e35-e4d6e7c7526b@suse.com> Date: Wed, 9 Aug 2017 10:49:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 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 08/08/17 22:09, Andy Lutomirski wrote: > On Tue, Aug 8, 2017 at 12:13 PM, Josh Poimboeuf wrote: >> On Tue, Aug 08, 2017 at 12:03:51PM -0700, Linus Torvalds wrote: >>> On Tue, Aug 8, 2017 at 11:58 AM, Josh Poimboeuf wrote: >>>> >>>> Take for example the lock_is_held_type() function. In vmlinux, it has >>>> the following instruction: >>>> >>>> callq *0xffffffff85a94880 (pv_irq_ops.save_fl) >>>> >>>> At runtime, that instruction is patched and replaced with a fast inline >>>> version of arch_local_save_flags() which eliminates the call: >>>> >>>> pushfq >>>> pop %rax >>>> >>>> The problem is when an interrupt hits after the push: >>>> >>>> pushfq >>>> --- irq --- >>>> pop %rax >>> >>> That should actually be something easily fixable, for an odd reason: >>> the instruction boundaries are different. >>> >>>> I'm not sure what the solution should be. It will probably need to be >>>> one of the following: >>>> >>>> a) either don't allow runtime "alternative" patches to mess with the >>>> stack pointer (objtool could enforce this); or >>>> >>>> b) come up with some way to register such patches with the ORC >>>> unwinder at runtime. >>> >>> c) just add ORC data for the alternative statically and _unconditionally_. >>> >>> No runtime registration. Just an unconditional entry for the >>> particular IP that comes after the "pushfq". It cannot match the >>> "callq" instruction, since it would be in the middle of that >>> instruction. >>> >>> Basically, just do a "union" of the ORC data for all the alternatives. >>> >>> Now, objtool should still verify that the instruction pointers for >>> alternatives are unique - or that they share the same ORC unwinder >>> information if they are not. >>> >>> But in cases like this, when the instruction boundaires are different, >>> things should "just work", with no need for any special cases. >>> >>> Hmm? >> >> Yeah, that might work. Objtool already knows about alternatives, so it >> might not be too hard. I'll try it. > > But this one's not an actual alternative, right? It's a pv op. > > I would advocate that we make it an alternative after all. I frickin' > hate the PV irq ops. It would like roughly like this: > > ALTERNATIVE "pushfq; popq %rax", "callq *pv_irq_ops.save_fl", > X86_FEATURE_GODDAMN_PV_IRQ_OPS You are aware that at least some of the Xen irq pvops functionality is patched inline? Your modification would slow down pv guests quite a bit, I guess. > (The obvious syntax error and the naming should probably be fixed. > Also, this needs to live in an #ifdef because it needs to build on > kernels with pv support. It should also properly register itself as a > pv patch site.) > > Semi-serious question: can we maybe delete lguest and 32-bit Xen PV > support some time soon? As far as I know, 32-bit Xen PV *hosts* are > all EOL and have no security support, 32-bit Xen PV guest dom0 may not > work (I've never tried, but it would certainly be nutty on a 64-bit > hypervisor), and lguest is, um, not seriously maintained any more. [1] Hmm, I suggested drop of lguest support about 3 months ago and got no response. OTOH there was no objection either. :-) Regarding 32 bit Xen PV guests: even 32 bit dom0 is supposed to work. In case you want to drop support in Linux you might ask that question on xen-devel@lists.xenproject.org (with PVH support for guests nearly complete your chances might be >0 to succeed). Juergen