From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752558AbdHJO7l (ORCPT ); Thu, 10 Aug 2017 10:59:41 -0400 Received: from mx2.suse.de ([195.135.220.15]:54812 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752162AbdHJO7j (ORCPT ); Thu, 10 Aug 2017 10:59:39 -0400 Subject: Re: [PATCH v4 1/2] x86/unwind: add ORC unwinder To: Josh Poimboeuf Cc: Peter Zijlstra , Andy Lutomirski , 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" , Mike Galbraith References: <79d6e04a-0a1d-592d-5e35-e4d6e7c7526b@suse.com> <20170809091648.p57ircm64t63mejj@hirez.programming.kicks-ass.net> <29fcd05f-3a0d-c20e-f20a-ee2019eea73c@suse.com> <20170809093511.oorxaiau5dtgfjxr@hirez.programming.kicks-ass.net> <20170809201506.izoiy3almhtnrljg@treble> <213d818c-0e15-3379-57c0-66035af28a71@suse.com> <20170810140903.3phgijen4w2tjp6w@treble> <20170810143910.y2wlb3btm5o2hxts@treble> From: Juergen Gross Message-ID: Date: Thu, 10 Aug 2017 16:59:36 +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: <20170810143910.y2wlb3btm5o2hxts@treble> 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 10/08/17 16:39, Josh Poimboeuf wrote: > On Thu, Aug 10, 2017 at 04:24:58PM +0200, Juergen Gross wrote: >>>> I'll send some patches to: >>>> >>>> - remove xen_patch() >>>> - remove lguest >>>> - remove vsmp >>>> >>>> In case nobody objects to apply those patches we can possibly simplify >>>> some more code. >>>> >>>> I'd love that. :-) >>> >>> Well, I might have spoken too soon about getting rid of vsmp. The >>> scalemp.com domain still exists. The code hasn't changed much in three >>> years, but maybe it's simple enough that it hasn't needed to change. >> >> Lets see. I have made the experience that asking whether some code can >> be removed almost never get answers. Sending a patch which actually >> removes the stuff results much more often in objections. :-) >> >>> Also, looking at the lguest mailing list, there seem to have been at >>> least a few people trying lguest out in the past year or so. >> >> Well, yes. The question is here whether there is a _need_ for lguest >> or was it just out of curiosity? >> >> In the end it is 32 bit only and you can easily test boot code via >> KVM, Xen or qemu. > > Good points. I'm all for removing code, so you have no objections from > me :-) > >>> Even if we couldn't get rid of vsmp or lguest, I wonder if the PVOP_CALL >>> stuff could be reworked to something like the following: >>> >>> static inline notrace unsigned long arch_local_save_flags(void) >>> { >>> return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl, >>> "pushfq; popq %rax", CPU_FEATURE_NATIVE, >>> "call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN, >>> "call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP, >>> "call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST); >>> } >>> >>> Which would eventually translate to something like: >>> >>> asm volatile(ALTERNATIVE_4("call *pv_irq_ops.save_fl", >>> "pushfq; popq %rax", CPU_FEATURE_NATIVE, >>> "call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN, >>> "call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP, >>> "call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST >>> : ... pvop clobber stuff ... ); >>> >>> where ALTERNATIVE_4 is a logical extension of ALTERNATIVE_2 and >>> CPU_FEATURE_NATIVE would always be set. >>> >>> It might need some more macro magic, but if it worked I think it would >>> be a lot clearer than the current voodoo. >>> >>> Thoughts? >> >> Hmm, this would modify the current approach of pvops completely: instead >> of letting each user of pvops (xen, lguest, vsmp, ...) set the functions >> it is needing, you'd have to modify the core definition of each pvops >> function for each user. > > Right. The callers (arch_local_save_flags, etc) would have to know > about the different hypervisors' functions. But this knowledge could be > hidden in inline functions and/or macros, so I don't see it being too > much of a problem. > > The upsides are that the behavior is much clearer (IMO), and we could > get rid of the .parainstructions stuff altogether. > >> Or would you want to let Xen, lguest etc. opt in >> for pvops and generate above code at build time from some templates? > > I'm not sure what you mean, can you clarify? It shouldn't be too much work to let each pvops user have a file in a common paravirt directory containing the needed information to create: static inline notrace unsigned long arch_local_save_flags(void) { return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl, "pushfq; popq %rax", CPU_FEATURE_NATIVE, "call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN, "call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP, "call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST); } and all other needed functions at build time. It could look e.g. like (for xen: xen.pv): @@feature CPU_FEATURE_XEN PV_IRQ_OPS_SAVE_FL "call __raw_callee_save_xen_save_fl" and the pre-processor could be used to assemble all configured users (pvops.pv): #ifdef CONFIG_XEN_PV #include "xen.pv" #endif #ifdef CONFIG_LGUEST #include "lguest.pv" #endif The resulting file would the be mangled by e.g. a python or awk script to a header containing macro definitions like: #define PV_IRQ_OPS_SAVE_FL \ "pushfq; popq %rax", CPU_FEATURE_NATIVE, \ "call __raw_callee_save_xen_save_fl", CPU_FEATURE_XEN, \ "call __raw_callee_save_vsmp_save_fl", CPU_FEATURE_VSMP, \ "call __raw_callee_save_lguest_save_fl", CPU_FEATURE_LGUEST which can then be used in paravirt.h: static inline notrace unsigned long arch_local_save_flags(void) { return PVOP_CALLEE0(unsigned long, pv_irq_ops.save_fl, PV_IRQ_OPS_SAVE_FL); } Juergen