From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3A521C433E3 for ; Tue, 14 Jul 2020 10:35:30 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0A7652073E for ; Tue, 14 Jul 2020 10:35:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="1UKiHauA"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="br/lLXnO" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0A7652073E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=j3QXo2c6KHm36C2krgT4uZH9xOHHEBAbAHGq7Wlc+3M=; b=1UKiHauArIr6CvueNUmB1R2H4 zAyKBAKjnK06cR1+6yqkZ/SM/+GNU1d0b4p19t48lWPdfkbdaWJw83e036uri4O6A+DUFHcRAl/Rz nXm5FkJsSjyFZpQOwl9RywFUyIutYdBHP4L59Rb1631EsY3I5bhl1ee5DkavpxuTWFx0Jj50lbwkI uc9Tfc+IfuQU3GkznPFUgCMSibO0GoG36obEvNTd1Rb+LHbNlgjQtZTiP4crdhZN76xU/l+WCEK9K ZtWjcdVOOo267oRx+4lgsPJNQHRGRseWb4uDmtKqSvyoubeU0ITqWD1GdjjWs2P5ox/8hjtRVy7gl DMeE7XXcg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jvIHT-0007Hf-5X; Tue, 14 Jul 2020 10:35:23 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jvIHP-0007G6-D0; Tue, 14 Jul 2020 10:35:20 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=HH5MekP+VtBs1CSslyllD2vFjpy9qUQDHcxcpFAt0Cg=; b=br/lLXnOwHnTXTnDX6To3QFoU 5NLL7twiv/zi55jQc2i6rniCYoe+CwuoZoXRLpMzlJ5B8LHXPP9rhENON1Sefkerm9r99IwNA6uNK z6J45dX+toA7Kn1Rx5vITCEYvxdk26KmmZpMD64niCJGWjRu8PFy0Lz3t5DGXbgBgqlFdHPsD2zxX HW5vAOevUYVjVN9dsAvNynv1uKWW2QnYNZOiZfViofIPFdlVqmbIrA7odZOJqba6UFSVQOd/Jn59v BMDnF37kNiRsSUJAso0l4OTQdduU7IjOt5FlVQik5hzAo5U5V/ukw9P0grSZdU3VN8dQynl1GYjeO 9+Jhfvk/w==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:39362) by pandora.armlinux.org.uk with esmtpsa (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jvIFv-00052s-E6; Tue, 14 Jul 2020 11:33:47 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.92) (envelope-from ) id 1jvIFh-0007E2-EL; Tue, 14 Jul 2020 11:33:33 +0100 Date: Tue, 14 Jul 2020 11:33:33 +0100 From: Russell King - ARM Linux admin To: Ard Biesheuvel Subject: Re: [PATCH 1/3] module: Rename module_alloc() to text_alloc() and move to kernel proper Message-ID: <20200714103333.GB1551@shell.armlinux.org.uk> References: <20200713182030.1418041-1-jarkko.sakkinen@linux.intel.com> <20200714095243.GB1442951@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200714_063519_672813_28D0CD9A X-CRM114-Status: GOOD ( 34.52 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Catalin Marinas , Kefeng Wang , Paul Mackerras , Zong Li , Andi Kleen , Paul Burton , Michael Ellerman , Vincent Whitchurch , Benjamin Herrenschmidt , Petr Mladek , Brian Gerst , Andy Lutomirski , Yonghong Song , Thomas Gleixner , Jiri Kosina , Anup Patel , Linux Kernel Mailing List , Philipp Rudo , Torsten Duwe , Masami Hiramatsu , Andrew Morton , Mark Rutland , "James E.J. Bottomley" , Vincent Chen , "open list:S390" , Joe Lawrence , Helge Deller , John Fastabend , Anil S Keshavamurthy , Andrey Ryabinin , Iurii Zaikin , Andrii Nakryiko , Vasily Gorbik , "moderated list:ARM PORT" , Daniel Axtens , Damien Le Moal , Peter Oberparleiter , Sean Christopherson , Martin KaFai Lau , Song Liu , Paul Walmsley , Heiko Carstens , Alexei Starovoitov , Jarkko Sakkinen , Atish Patra , Will Deacon , Daniel Borkmann , Masahiro Yamada , Nayna Jain , Ley Foon Tan , Christian Borntraeger , Dmitry Vyukov , Sami Tolvanen , "Naveen N. Rao" , Mao Han , Marco Elver , Steven Rostedt , Babu Moger , Borislav Petkov , Greentime Hu , Ben Dooks , Guan Xuetao , Thomas Bogendoerfer , "open list:PARISC ARCHITECTURE" , Jessica Yu , "open list:BPF JIT for MIPS \(32-BIT AND 64-BIT\)" , "David S. Miller" , Thiago Jung Bauermann , Peter Zijlstra , "open list:SPARC + UltraSPARC \(sparc/sparc64\)" , Sandipan Das , "H. Peter Anvin" , Amit Daniel Kachhap , Tiezhu Yang , Miroslav Benes , Jiri Olsa , "open list:RISC-V ARCHITECTURE" , Vincenzo Frascino , Anders Roxell , Sven Schnelle , "maintainer:X86 ARCHITECTURE \(32-BIT AND 64-BIT\)" , Mike Rapoport , Ingo Molnar , Albert Ou , "Paul E. McKenney" , Josh Poimboeuf , KP Singh , Gerald Schaefer , Nick Hu , "open list:BPF JIT for MIPS \(32-BIT AND 64-BIT\)" , "open list:MIPS" , Sergey Senozhatsky , Palmer Dabbelt , "open list:LINUX FOR POWERPC \(32-BIT AND 64-BIT\)" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Tue, Jul 14, 2020 at 01:17:22PM +0300, Ard Biesheuvel wrote: > On Tue, 14 Jul 2020 at 12:53, Jarkko Sakkinen > wrote: > > > > On Mon, Jul 13, 2020 at 10:49:48PM +0300, Ard Biesheuvel wrote: > > > This patch suggests that there are other reasons why conflating > > > allocation of module space and allocating text pages for other uses > > > is a bad idea, but switching all users to text_alloc() is a step in > > > the wrong direction. It would be better to stop using module_alloc() > > > in core code except in the module loader, and have a generic > > > text_alloc() that can be overridden by the arch if necessary. Note > > > that x86 and s390 are the only architectures that use module_alloc() > > > in ftrace code. > > > > This series essentially does this: introduces text_alloc() and > > text_memfree(), which have generic implementations in kernel/text.c. > > Those can be overriddent by arch specific implementations. > > > > What you think should be done differently than in my patch set? > > > > On arm64, module_alloc is only used by the module loader, and so > pulling it out and renaming it will cause unused code to be > incorporated into the kernel when building without module support, > which is the use case you claim to be addressing. > > Module_alloc has semantics that are intimately tied to the module > loader, but over the years, it ended up being (ab)used by other > subsystems, which don't require those semantics but just need n pages > of vmalloc space with executable permissions. > > So the correct approach is to make text_alloc() implement just that, > generically, and switch bpf etc to use it. Then, only on architectures > that need it, override it with an implementation that has the required > additional semantics. > > Refactoring 10+ architectures like this without any regard for how > text_alloc() deviates from module_alloc() just creates a lot of churn > that others will have to clean up after you. For 32-bit ARM, our bpf code uses "blx/bx" (or equivalent code sequences) rather than encoding a "bl" or "b", so BPF there doesn't care where the executable memory is mapped, and doesn't need any PLTs. Given that, should bpf always allocate from the vmalloc() region to preserve the module space for modules? I'm more concerned about ftrace though, but only because I don't have the understanding of that subsystem to really say whether there are any side effects from having the allocations potentially be out of range of a "bl" or "b" instruction. If ftrace jumps both to and from the allocated page using a "load address to register, branch to register" approach like BPF does, then ftrace should be safe - and again, raises the issue that maybe it should always come from vmalloc() space. So, I think we need to keep module_alloc() for allocating memory for modules, and provide a new text_alloc() for these other cases. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv