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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BDBB4C433EF for ; Wed, 25 May 2022 22:25:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239940AbiEYWZp (ORCPT ); Wed, 25 May 2022 18:25:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54820 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234993AbiEYWZo (ORCPT ); Wed, 25 May 2022 18:25:44 -0400 Received: from mail-lj1-x236.google.com (mail-lj1-x236.google.com [IPv6:2a00:1450:4864:20::236]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 94721BA9A9 for ; Wed, 25 May 2022 15:25:42 -0700 (PDT) Received: by mail-lj1-x236.google.com with SMTP id q1so16332895ljb.5 for ; Wed, 25 May 2022 15:25:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Y4JE5dVY5pJ4L5p/4sGg1xocj3NSEFAKSZJJs4bfl50=; b=fAMfron/HjbN7nd+bsG3ASO/nK1WjlcoVqa4+QdtPam6ciW/bwdYCmk9ujS4Abe4VJ w2sueG/5d5jh11UgLmuXnMAdarDjSChSdXdKrhg3y7F8Pjptb4ff8WlByIdiSkfEbGFm PkRmC+09uBSPSsg344wCJdI8n7v1Yt+vr7kQ9DePitEy6ui34PJ0AU8C5jZKRsffEDfP ug3LM0AmYwwUZHhVNWlMxQzBz79rShyO1d1dkhvX1r4lmvgXBfWQm+lShZOQXq9rxarC SWsS9kfyfYspj/5U44fShRnUcSFmlbceeiOtALpaquQdvDHDNyVkshBPqlQYpF3P0oqm UHfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Y4JE5dVY5pJ4L5p/4sGg1xocj3NSEFAKSZJJs4bfl50=; b=rUCLGdxpuqdiJfD8dEnrD2e9qncJzhvTalcqalSBXz2P7RR/IsOsvV6/lQixHyPn3T b1eQC+HRG2526iaQIdWdweqGKhSLP96Drff5geAbmen9GMhkTqgZfps+KxYUL1+yFu1M gZLnYhSw5m7ymG3+Eu3kKWxmwXWCXppqAHg0hvshZioS460OTOnCKE+K342GKAomKRCw ynLaa7rkuHtupknbdyNOumKb1VhGY8OXDe+72+tsWtp6oaYtyqmMIOv5C2ZMm3LS7NGx pIg5m+TV4oI+1QipQGf3pZcFI/fy4y6rC0XCIYFnLdZViKXTeGk6poNhcltrm/+YMTgB 1+kQ== X-Gm-Message-State: AOAM533DhtcJUgoky62MopML9elop/stiF4HLSs4eNCt5VX/Zle53vRx Ta2awb3Dla7q7PAv4kOK4cHdImzAaWg2uxJeURuYUQ== X-Google-Smtp-Source: ABdhPJyyMkDOBVLKOsbECK6J7ZWww0BuYU20TDN6fYXnxk8xptFKTQXny9GfJRyE8hkAsArQFcu6jZmKOkIJSKODNlQ= X-Received: by 2002:a2e:98c3:0:b0:253:e0e1:20 with SMTP id s3-20020a2e98c3000000b00253e0e10020mr14949440ljj.26.1653517540546; Wed, 25 May 2022 15:25:40 -0700 (PDT) MIME-Version: 1.0 References: <20220523020209.11810-1-ojeda@kernel.org> <20220523020209.11810-22-ojeda@kernel.org> In-Reply-To: <20220523020209.11810-22-ojeda@kernel.org> From: Nick Desaulniers Date: Wed, 25 May 2022 15:25:28 -0700 Message-ID: Subject: Re: [PATCH v7 21/25] Kbuild: add Rust support To: Miguel Ojeda , Masahiro Yamada Cc: Linus Torvalds , Greg Kroah-Hartman , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Jarkko Sakkinen , Alex Gaynor , Finn Behrens , Adam Bratschi-Kaye , Wedson Almeida Filho , Michael Ellerman , Sven Van Asbroeck , Gary Guo , Boris-Chengbiao Zhou , Boqun Feng , Douglas Su , Dariusz Sosnowski , Antonio Terceiro , Daniel Xu , Miguel Cano , David Gow , Michal Marek , Russell King , Catalin Marinas , Will Deacon , Benjamin Herrenschmidt , Paul Mackerras , Paul Walmsley , Palmer Dabbelt , Albert Ou , Richard Weinberger , Anton Ivanov , Johannes Berg , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , linux-kbuild@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-um@lists.infradead.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: rust-for-linux@vger.kernel.org On Sun, May 22, 2022 at 7:04 PM Miguel Ojeda wrote: > > --- a/Makefile > +++ b/Makefile > @@ -120,6 +120,15 @@ endif > > export KBUILD_CHECKSRC > > +# Enable "clippy" (a linter) as part of the Rust compilation. > +# > +# Use 'make CLIPPY=1' to enable it. > +ifeq ("$(origin CLIPPY)", "command line") > + KBUILD_CLIPPY := $(CLIPPY) > +endif Is there a reason to not just turn clippy on always? Might be nicer to start off with good practices by default. :^) > @@ -1494,7 +1588,8 @@ MRPROPER_FILES += include/config include/generated \ > certs/signing_key.pem \ > certs/x509.genkey \ > vmlinux-gdb.py \ > - *.spec > + *.spec \ > + rust/target.json rust/libmacros.so > > # clean - Delete most, but leave enough to build external modules > # > @@ -1519,6 +1614,9 @@ $(mrproper-dirs): > > mrproper: clean $(mrproper-dirs) > $(call cmd,rmfiles) > + @find . $(RCS_FIND_IGNORE) \ > + \( -name '*.rmeta' \) \ > + -type f -print | xargs rm -f Are there *.rmeta directories that we _don't_ want to remove via `make mrproper`? I'm curious why *.rmeta isn't just part of MRPROPER_FILES? > > # distclean > # > @@ -1606,6 +1704,23 @@ help: > @echo ' kselftest-merge - Merge all the config dependencies of' > @echo ' kselftest to existing .config.' > @echo '' > + @echo 'Rust targets:' > + @echo ' rustavailable - Checks whether the Rust toolchain is' > + @echo ' available and, if not, explains why.' > + @echo ' rustfmt - Reformat all the Rust code in the kernel' > + @echo ' rustfmtcheck - Checks if all the Rust code in the kernel' > + @echo ' is formatted, printing a diff otherwise.' > + @echo ' rustdoc - Generate Rust documentation' > + @echo ' (requires kernel .config)' > + @echo ' rusttest - Runs the Rust tests' > + @echo ' (requires kernel .config; downloads external repos)' > + @echo ' rust-analyzer - Generate rust-project.json rust-analyzer support file' > + @echo ' (requires kernel .config)' > + @echo ' dir/file.[os] - Build specified target only' > + @echo ' dir/file.i - Build macro expanded source, similar to C preprocessing' > + @echo ' (run with RUSTFMT=n to skip reformatting if needed)' > + @echo ' dir/file.ll - Build the LLVM assembly file' I don't think we need to repeat dir/* here again for rust. The existing targets listed above (outside this hunk) make sense in both contexts. Does rustc really use .i as a conventional suffix for macro expanded sources? (The C compiler might use the -x flag to override the guess it would make based on the file extension; I'm curious if rustc can ingest .i files or will it warn?) > diff --git a/init/Kconfig b/init/Kconfig > index ddcbefe535e9..3457cf596588 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > +config RUSTC_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(RUSTC) >/dev/null 2>&1 && $(RUSTC) --version || echo n) > + > +config BINDGEN_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(BINDGEN) >/dev/null 2>&1 && $(BINDGEN) --version || echo n) Are these two kconfigs used anywhere? > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > index 0496efd6e117..83e850321eb6 100644 > --- a/scripts/Kconfig.include > +++ b/scripts/Kconfig.include > @@ -36,12 +36,12 @@ ld-option = $(success,$(LD) -v $(1)) > as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -) > > # check if $(CC) and $(LD) exist > -$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found) > +$(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found) Not that it's important to do so, but a couple hunks s/compiler/C compiler/. Those _could_ probably get submitted ahead of this, but not important to do so. > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index 9f39b0130551..fe87389d52c0 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -1,4 +1,5 @@ > DEBUG_CFLAGS := > +DEBUG_RUSTFLAGS := > > ifdef CONFIG_DEBUG_INFO_SPLIT > DEBUG_CFLAGS += -gsplit-dwarf > @@ -10,6 +11,12 @@ ifndef CONFIG_AS_IS_LLVM > KBUILD_AFLAGS += -Wa,-gdwarf-2 > endif > > +ifdef CONFIG_DEBUG_INFO_REDUCED > +DEBUG_RUSTFLAGS += -Cdebuginfo=1 > +else > +DEBUG_RUSTFLAGS += -Cdebuginfo=2 > +endif > + How does enabling or disabling debug info work for rustc? I may have missed it, but I was surprised to see no additional flags getting passed to rustc based on CONFIG_DEBUG info. > diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs > new file mode 100644 > index 000000000000..c146a3407183 > --- /dev/null > +++ b/scripts/generate_rust_target.rs Ah, that explains the host rust build infra. Bravo! Hard coding the target files was my major concern last I looked at the series. I'm very happy to see it be generated properly from .config! I haven't actually reviewed this yet, but it makes me significantly more confident in the series to see this approach added. Good job whoever wrote this. > diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh > new file mode 100755 > index 000000000000..277a64d07f22 > --- /dev/null > +++ b/scripts/is_rust_module.sh > @@ -0,0 +1,13 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# is_rust_module.sh module.ko > +# > +# Returns `0` if `module.ko` is a Rust module, `1` otherwise. > + > +set -e > + > +# Using the `16_` prefix ensures other symbols with the same substring > +# are not picked up (even if it would be unlikely). The last part is > +# used just in case LLVM decides to use the `.` suffix. > +${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$' Does `$(READELF) -p .comment foo.o` print anything about which compiler was used? That seems less brittle IMO. --- Modulo the RUST_OPT_LEVEL_SIMILAR_AS_CHOSEN_FOR_C which I'm not a fan of, this is looking good to me. Masahiro, what are your thoughts on the build system support? -- Thanks, ~Nick Desaulniers 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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 672BDC433EF for ; Wed, 25 May 2022 22:25:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=AB+6IvbslamD366yd2VLszyD4bDadwQVs2OrjwW1xIg=; b=xBJh6TNV1vQhRk hq7gOx7W43C8Qr9Tu4je8CEgdrDHuc73FTIcB6uSiXNXAlVM3del5+TiKV1CYWir/aSOktziQ77Vn BQlrgzQV0aiRd2qCRAKqptvEa3DJ603JtXh9HQtJjf01Dif0vbl5UcXbj6pTOQVk/HGg/ZwPwqp8R 1E8YCZkWKi6aGATJtrPMoThoUDqe0ou4WFbeRnOf8hVXeE+PiqZ/FCF6elgXzHLYuKfdYx6oloFnp wnoTjGbcSjN3MY7AEXXHluMipIy5rzYGLivV9zUZmemeEpvqFkF/rn4dJaePObxN8VxPlfQuni0KN tILURET9rtKtZomxQrIA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntzRq-00Crt4-Ev; Wed, 25 May 2022 22:25:46 +0000 Received: from mail-lj1-x231.google.com ([2a00:1450:4864:20::231]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntzRn-00Crs3-IA for linux-riscv@lists.infradead.org; Wed, 25 May 2022 22:25:45 +0000 Received: by mail-lj1-x231.google.com with SMTP id s20so4230866ljd.10 for ; Wed, 25 May 2022 15:25:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Y4JE5dVY5pJ4L5p/4sGg1xocj3NSEFAKSZJJs4bfl50=; b=fAMfron/HjbN7nd+bsG3ASO/nK1WjlcoVqa4+QdtPam6ciW/bwdYCmk9ujS4Abe4VJ w2sueG/5d5jh11UgLmuXnMAdarDjSChSdXdKrhg3y7F8Pjptb4ff8WlByIdiSkfEbGFm PkRmC+09uBSPSsg344wCJdI8n7v1Yt+vr7kQ9DePitEy6ui34PJ0AU8C5jZKRsffEDfP ug3LM0AmYwwUZHhVNWlMxQzBz79rShyO1d1dkhvX1r4lmvgXBfWQm+lShZOQXq9rxarC SWsS9kfyfYspj/5U44fShRnUcSFmlbceeiOtALpaquQdvDHDNyVkshBPqlQYpF3P0oqm UHfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Y4JE5dVY5pJ4L5p/4sGg1xocj3NSEFAKSZJJs4bfl50=; b=7XhrngxigP4x+hMUXug9yYp2YX/OLJ1IPc2wJ9AeQIKFgQ84qQpbIDsXtmDg5W0mFG iHVkTx2S4TfQg9XMzpEPDbelKh+eFIDmkXe+MvCXRQD5CV0AJ0l+FYidkmFLgV6fryPF g3tSJnB21H8phZOuFRRDQp/q+GAQHNWydxOvrQplooYRFg+y13SCXG5ylfzowXza1K0D DddD6c3BaA0OSqwEaIYPLTGr70s7Tplndqx+0XGHASafn6Az3sivH+itlDSoqBeirJMO ZyFhffMV5gGx6sRvf5vnb36x4LVzDBA5v02D16D9TBTYALI4jAG68LRy/JTeAyxlOmgp GclA== X-Gm-Message-State: AOAM5314l9E3x8jsghoO4uc4T9ld+FQl06cV+1HjToF7oPNvsuL/7gHs 8PVY7IfIVK+EpAnlOQvJcpfjtmCp92OdOJYA+ocRvw== X-Google-Smtp-Source: ABdhPJyyMkDOBVLKOsbECK6J7ZWww0BuYU20TDN6fYXnxk8xptFKTQXny9GfJRyE8hkAsArQFcu6jZmKOkIJSKODNlQ= X-Received: by 2002:a2e:98c3:0:b0:253:e0e1:20 with SMTP id s3-20020a2e98c3000000b00253e0e10020mr14949440ljj.26.1653517540546; Wed, 25 May 2022 15:25:40 -0700 (PDT) MIME-Version: 1.0 References: <20220523020209.11810-1-ojeda@kernel.org> <20220523020209.11810-22-ojeda@kernel.org> In-Reply-To: <20220523020209.11810-22-ojeda@kernel.org> From: Nick Desaulniers Date: Wed, 25 May 2022 15:25:28 -0700 Message-ID: Subject: Re: [PATCH v7 21/25] Kbuild: add Rust support To: Miguel Ojeda , Masahiro Yamada Cc: Linus Torvalds , Greg Kroah-Hartman , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Jarkko Sakkinen , Alex Gaynor , Finn Behrens , Adam Bratschi-Kaye , Wedson Almeida Filho , Michael Ellerman , Sven Van Asbroeck , Gary Guo , Boris-Chengbiao Zhou , Boqun Feng , Douglas Su , Dariusz Sosnowski , Antonio Terceiro , Daniel Xu , Miguel Cano , David Gow , Michal Marek , Russell King , Catalin Marinas , Will Deacon , Benjamin Herrenschmidt , Paul Mackerras , Paul Walmsley , Palmer Dabbelt , Albert Ou , Richard Weinberger , Anton Ivanov , Johannes Berg , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , linux-kbuild@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-um@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220525_152543_665725_5AF74932 X-CRM114-Status: GOOD ( 32.55 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Sun, May 22, 2022 at 7:04 PM Miguel Ojeda wrote: > > --- a/Makefile > +++ b/Makefile > @@ -120,6 +120,15 @@ endif > > export KBUILD_CHECKSRC > > +# Enable "clippy" (a linter) as part of the Rust compilation. > +# > +# Use 'make CLIPPY=1' to enable it. > +ifeq ("$(origin CLIPPY)", "command line") > + KBUILD_CLIPPY := $(CLIPPY) > +endif Is there a reason to not just turn clippy on always? Might be nicer to start off with good practices by default. :^) > @@ -1494,7 +1588,8 @@ MRPROPER_FILES += include/config include/generated \ > certs/signing_key.pem \ > certs/x509.genkey \ > vmlinux-gdb.py \ > - *.spec > + *.spec \ > + rust/target.json rust/libmacros.so > > # clean - Delete most, but leave enough to build external modules > # > @@ -1519,6 +1614,9 @@ $(mrproper-dirs): > > mrproper: clean $(mrproper-dirs) > $(call cmd,rmfiles) > + @find . $(RCS_FIND_IGNORE) \ > + \( -name '*.rmeta' \) \ > + -type f -print | xargs rm -f Are there *.rmeta directories that we _don't_ want to remove via `make mrproper`? I'm curious why *.rmeta isn't just part of MRPROPER_FILES? > > # distclean > # > @@ -1606,6 +1704,23 @@ help: > @echo ' kselftest-merge - Merge all the config dependencies of' > @echo ' kselftest to existing .config.' > @echo '' > + @echo 'Rust targets:' > + @echo ' rustavailable - Checks whether the Rust toolchain is' > + @echo ' available and, if not, explains why.' > + @echo ' rustfmt - Reformat all the Rust code in the kernel' > + @echo ' rustfmtcheck - Checks if all the Rust code in the kernel' > + @echo ' is formatted, printing a diff otherwise.' > + @echo ' rustdoc - Generate Rust documentation' > + @echo ' (requires kernel .config)' > + @echo ' rusttest - Runs the Rust tests' > + @echo ' (requires kernel .config; downloads external repos)' > + @echo ' rust-analyzer - Generate rust-project.json rust-analyzer support file' > + @echo ' (requires kernel .config)' > + @echo ' dir/file.[os] - Build specified target only' > + @echo ' dir/file.i - Build macro expanded source, similar to C preprocessing' > + @echo ' (run with RUSTFMT=n to skip reformatting if needed)' > + @echo ' dir/file.ll - Build the LLVM assembly file' I don't think we need to repeat dir/* here again for rust. The existing targets listed above (outside this hunk) make sense in both contexts. Does rustc really use .i as a conventional suffix for macro expanded sources? (The C compiler might use the -x flag to override the guess it would make based on the file extension; I'm curious if rustc can ingest .i files or will it warn?) > diff --git a/init/Kconfig b/init/Kconfig > index ddcbefe535e9..3457cf596588 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > +config RUSTC_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(RUSTC) >/dev/null 2>&1 && $(RUSTC) --version || echo n) > + > +config BINDGEN_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(BINDGEN) >/dev/null 2>&1 && $(BINDGEN) --version || echo n) Are these two kconfigs used anywhere? > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > index 0496efd6e117..83e850321eb6 100644 > --- a/scripts/Kconfig.include > +++ b/scripts/Kconfig.include > @@ -36,12 +36,12 @@ ld-option = $(success,$(LD) -v $(1)) > as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -) > > # check if $(CC) and $(LD) exist > -$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found) > +$(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found) Not that it's important to do so, but a couple hunks s/compiler/C compiler/. Those _could_ probably get submitted ahead of this, but not important to do so. > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index 9f39b0130551..fe87389d52c0 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -1,4 +1,5 @@ > DEBUG_CFLAGS := > +DEBUG_RUSTFLAGS := > > ifdef CONFIG_DEBUG_INFO_SPLIT > DEBUG_CFLAGS += -gsplit-dwarf > @@ -10,6 +11,12 @@ ifndef CONFIG_AS_IS_LLVM > KBUILD_AFLAGS += -Wa,-gdwarf-2 > endif > > +ifdef CONFIG_DEBUG_INFO_REDUCED > +DEBUG_RUSTFLAGS += -Cdebuginfo=1 > +else > +DEBUG_RUSTFLAGS += -Cdebuginfo=2 > +endif > + How does enabling or disabling debug info work for rustc? I may have missed it, but I was surprised to see no additional flags getting passed to rustc based on CONFIG_DEBUG info. > diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs > new file mode 100644 > index 000000000000..c146a3407183 > --- /dev/null > +++ b/scripts/generate_rust_target.rs Ah, that explains the host rust build infra. Bravo! Hard coding the target files was my major concern last I looked at the series. I'm very happy to see it be generated properly from .config! I haven't actually reviewed this yet, but it makes me significantly more confident in the series to see this approach added. Good job whoever wrote this. > diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh > new file mode 100755 > index 000000000000..277a64d07f22 > --- /dev/null > +++ b/scripts/is_rust_module.sh > @@ -0,0 +1,13 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# is_rust_module.sh module.ko > +# > +# Returns `0` if `module.ko` is a Rust module, `1` otherwise. > + > +set -e > + > +# Using the `16_` prefix ensures other symbols with the same substring > +# are not picked up (even if it would be unlikely). The last part is > +# used just in case LLVM decides to use the `.` suffix. > +${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$' Does `$(READELF) -p .comment foo.o` print anything about which compiler was used? That seems less brittle IMO. --- Modulo the RUST_OPT_LEVEL_SIMILAR_AS_CHOSEN_FOR_C which I'm not a fan of, this is looking good to me. Masahiro, what are your thoughts on the build system support? -- Thanks, ~Nick Desaulniers _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv 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 Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CC864C433F5 for ; Wed, 25 May 2022 22:26:26 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4L7lz51yx3z3bpy for ; Thu, 26 May 2022 08:26:25 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=fAMfron/; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=google.com (client-ip=2a00:1450:4864:20::231; helo=mail-lj1-x231.google.com; envelope-from=ndesaulniers@google.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=fAMfron/; dkim-atps=neutral Received: from mail-lj1-x231.google.com (mail-lj1-x231.google.com [IPv6:2a00:1450:4864:20::231]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4L7lyP42ySz2yXP for ; Thu, 26 May 2022 08:25:48 +1000 (AEST) Received: by mail-lj1-x231.google.com with SMTP id i23so26008886ljb.4 for ; Wed, 25 May 2022 15:25:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Y4JE5dVY5pJ4L5p/4sGg1xocj3NSEFAKSZJJs4bfl50=; b=fAMfron/HjbN7nd+bsG3ASO/nK1WjlcoVqa4+QdtPam6ciW/bwdYCmk9ujS4Abe4VJ w2sueG/5d5jh11UgLmuXnMAdarDjSChSdXdKrhg3y7F8Pjptb4ff8WlByIdiSkfEbGFm PkRmC+09uBSPSsg344wCJdI8n7v1Yt+vr7kQ9DePitEy6ui34PJ0AU8C5jZKRsffEDfP ug3LM0AmYwwUZHhVNWlMxQzBz79rShyO1d1dkhvX1r4lmvgXBfWQm+lShZOQXq9rxarC SWsS9kfyfYspj/5U44fShRnUcSFmlbceeiOtALpaquQdvDHDNyVkshBPqlQYpF3P0oqm UHfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Y4JE5dVY5pJ4L5p/4sGg1xocj3NSEFAKSZJJs4bfl50=; b=5aZ3B09Z8nrx7g9SnbbbbBu+7wOFBpZU04PcVaaDpRtwrU3b1i6OJraln2lQdlZk7I GUtw3wuhbrrKqLZGXrU1vJgXQeo/siRMDXsEPE5t1e6LKx4Ch960a63/iarjF/b5E4Hc kZUxRBWBg9Bd/ke3VbjBl3fUuZ9gDLxcApeZovzDCTWQl4EhRdUeQId/AtnRr5eRPTUM rHZwWU3nSziMWhEih6mVcK3U/EwUEqCTWIdmaEg6A5gMqCDDEsd/r0EVukfBLfgrwpcz 6k/F066ya4UMketwb85qk3yTAAn32g/uWXIjQm+O0SGxD5uXccIwOjfS8W2LIRQRGmxh 8apw== X-Gm-Message-State: AOAM530j0tHJp9uS08C0GswDCVw/VwQqRgqnSZn3KfyUKsMU0ur7A/Tk itj8MEpK9OnqhM39EyIoOy/Zz756A7s3vxDTA7opWg== X-Google-Smtp-Source: ABdhPJyyMkDOBVLKOsbECK6J7ZWww0BuYU20TDN6fYXnxk8xptFKTQXny9GfJRyE8hkAsArQFcu6jZmKOkIJSKODNlQ= X-Received: by 2002:a2e:98c3:0:b0:253:e0e1:20 with SMTP id s3-20020a2e98c3000000b00253e0e10020mr14949440ljj.26.1653517540546; Wed, 25 May 2022 15:25:40 -0700 (PDT) MIME-Version: 1.0 References: <20220523020209.11810-1-ojeda@kernel.org> <20220523020209.11810-22-ojeda@kernel.org> In-Reply-To: <20220523020209.11810-22-ojeda@kernel.org> From: Nick Desaulniers Date: Wed, 25 May 2022 15:25:28 -0700 Message-ID: Subject: Re: [PATCH v7 21/25] Kbuild: add Rust support To: Miguel Ojeda , Masahiro Yamada Content-Type: text/plain; charset="UTF-8" X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Sven Van Asbroeck , Catalin Marinas , Dave Hansen , Miguel Cano , Paul Mackerras , Gary Guo , Douglas Su , Borislav Petkov , linux-riscv@lists.infradead.org, Will Deacon , Thomas Gleixner , Anton Ivanov , "H. Peter Anvin" , x86@kernel.org, Russell King , Ingo Molnar , Wedson Almeida Filho , Alex Gaynor , Antonio Terceiro , Adam Bratschi-Kaye , Albert Ou , rust-for-linux@vger.kernel.org, linux-kbuild@vger.kernel.org, Boqun Feng , linux-um@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, Daniel Xu , David Gow , Paul Walmsley , Dariusz Sosnowski , linux-arm-kernel@lists.infradead.org, Michal Marek , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Boris-Chengbiao Zhou , Jarkko Sakkinen , Palmer Dabbelt , Richard Weinberger , Finn Behrens , Johannes Berg , Linus Torvalds Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Sun, May 22, 2022 at 7:04 PM Miguel Ojeda wrote: > > --- a/Makefile > +++ b/Makefile > @@ -120,6 +120,15 @@ endif > > export KBUILD_CHECKSRC > > +# Enable "clippy" (a linter) as part of the Rust compilation. > +# > +# Use 'make CLIPPY=1' to enable it. > +ifeq ("$(origin CLIPPY)", "command line") > + KBUILD_CLIPPY := $(CLIPPY) > +endif Is there a reason to not just turn clippy on always? Might be nicer to start off with good practices by default. :^) > @@ -1494,7 +1588,8 @@ MRPROPER_FILES += include/config include/generated \ > certs/signing_key.pem \ > certs/x509.genkey \ > vmlinux-gdb.py \ > - *.spec > + *.spec \ > + rust/target.json rust/libmacros.so > > # clean - Delete most, but leave enough to build external modules > # > @@ -1519,6 +1614,9 @@ $(mrproper-dirs): > > mrproper: clean $(mrproper-dirs) > $(call cmd,rmfiles) > + @find . $(RCS_FIND_IGNORE) \ > + \( -name '*.rmeta' \) \ > + -type f -print | xargs rm -f Are there *.rmeta directories that we _don't_ want to remove via `make mrproper`? I'm curious why *.rmeta isn't just part of MRPROPER_FILES? > > # distclean > # > @@ -1606,6 +1704,23 @@ help: > @echo ' kselftest-merge - Merge all the config dependencies of' > @echo ' kselftest to existing .config.' > @echo '' > + @echo 'Rust targets:' > + @echo ' rustavailable - Checks whether the Rust toolchain is' > + @echo ' available and, if not, explains why.' > + @echo ' rustfmt - Reformat all the Rust code in the kernel' > + @echo ' rustfmtcheck - Checks if all the Rust code in the kernel' > + @echo ' is formatted, printing a diff otherwise.' > + @echo ' rustdoc - Generate Rust documentation' > + @echo ' (requires kernel .config)' > + @echo ' rusttest - Runs the Rust tests' > + @echo ' (requires kernel .config; downloads external repos)' > + @echo ' rust-analyzer - Generate rust-project.json rust-analyzer support file' > + @echo ' (requires kernel .config)' > + @echo ' dir/file.[os] - Build specified target only' > + @echo ' dir/file.i - Build macro expanded source, similar to C preprocessing' > + @echo ' (run with RUSTFMT=n to skip reformatting if needed)' > + @echo ' dir/file.ll - Build the LLVM assembly file' I don't think we need to repeat dir/* here again for rust. The existing targets listed above (outside this hunk) make sense in both contexts. Does rustc really use .i as a conventional suffix for macro expanded sources? (The C compiler might use the -x flag to override the guess it would make based on the file extension; I'm curious if rustc can ingest .i files or will it warn?) > diff --git a/init/Kconfig b/init/Kconfig > index ddcbefe535e9..3457cf596588 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > +config RUSTC_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(RUSTC) >/dev/null 2>&1 && $(RUSTC) --version || echo n) > + > +config BINDGEN_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(BINDGEN) >/dev/null 2>&1 && $(BINDGEN) --version || echo n) Are these two kconfigs used anywhere? > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > index 0496efd6e117..83e850321eb6 100644 > --- a/scripts/Kconfig.include > +++ b/scripts/Kconfig.include > @@ -36,12 +36,12 @@ ld-option = $(success,$(LD) -v $(1)) > as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -) > > # check if $(CC) and $(LD) exist > -$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found) > +$(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found) Not that it's important to do so, but a couple hunks s/compiler/C compiler/. Those _could_ probably get submitted ahead of this, but not important to do so. > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index 9f39b0130551..fe87389d52c0 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -1,4 +1,5 @@ > DEBUG_CFLAGS := > +DEBUG_RUSTFLAGS := > > ifdef CONFIG_DEBUG_INFO_SPLIT > DEBUG_CFLAGS += -gsplit-dwarf > @@ -10,6 +11,12 @@ ifndef CONFIG_AS_IS_LLVM > KBUILD_AFLAGS += -Wa,-gdwarf-2 > endif > > +ifdef CONFIG_DEBUG_INFO_REDUCED > +DEBUG_RUSTFLAGS += -Cdebuginfo=1 > +else > +DEBUG_RUSTFLAGS += -Cdebuginfo=2 > +endif > + How does enabling or disabling debug info work for rustc? I may have missed it, but I was surprised to see no additional flags getting passed to rustc based on CONFIG_DEBUG info. > diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs > new file mode 100644 > index 000000000000..c146a3407183 > --- /dev/null > +++ b/scripts/generate_rust_target.rs Ah, that explains the host rust build infra. Bravo! Hard coding the target files was my major concern last I looked at the series. I'm very happy to see it be generated properly from .config! I haven't actually reviewed this yet, but it makes me significantly more confident in the series to see this approach added. Good job whoever wrote this. > diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh > new file mode 100755 > index 000000000000..277a64d07f22 > --- /dev/null > +++ b/scripts/is_rust_module.sh > @@ -0,0 +1,13 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# is_rust_module.sh module.ko > +# > +# Returns `0` if `module.ko` is a Rust module, `1` otherwise. > + > +set -e > + > +# Using the `16_` prefix ensures other symbols with the same substring > +# are not picked up (even if it would be unlikely). The last part is > +# used just in case LLVM decides to use the `.` suffix. > +${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$' Does `$(READELF) -p .comment foo.o` print anything about which compiler was used? That seems less brittle IMO. --- Modulo the RUST_OPT_LEVEL_SIMILAR_AS_CHOSEN_FOR_C which I'm not a fan of, this is looking good to me. Masahiro, what are your thoughts on the build system support? -- Thanks, ~Nick Desaulniers 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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 83F9EC433EF for ; Wed, 25 May 2022 22:27:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ZZZuSgL8DX32eTOswvnk6S94sduDLpAFbnIMr7pgL3U=; b=JEVcdD75YqhwLd T2wvWB7KQ/ZI4JehUW06380KnugJpgGN0cKA1RgWYREqQdIkr6WKsLarh2Il2QgW4UYl9cE4gqveW QCgFJIaKN8NctNXTSYjk/+vDWXbajhVPB1SzTA1KGl1zfJYoXLDrIJ3qr72otMu7EhGujA0G4neKb EClquK13Nm5z5yJbHyKi6qWZDRjTkuDjWFyhePuUXUfshG5weDu4hPHlatsq7SBwDclDuIP8c+HAX 5aBHH49GzSpENSTagAGAinqxbKO2YEoRQo+y4nkM0rb9JgRcSOd+zWHBJ8pqu4B1R3W3z92G1l1u1 o1ajGNPSa7lveQ7FJshw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntzRs-00Crtk-Lh; Wed, 25 May 2022 22:25:48 +0000 Received: from mail-lj1-x235.google.com ([2a00:1450:4864:20::235]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntzRo-00Crs2-NO for linux-arm-kernel@lists.infradead.org; Wed, 25 May 2022 22:25:46 +0000 Received: by mail-lj1-x235.google.com with SMTP id a23so25972489ljd.9 for ; Wed, 25 May 2022 15:25:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Y4JE5dVY5pJ4L5p/4sGg1xocj3NSEFAKSZJJs4bfl50=; b=fAMfron/HjbN7nd+bsG3ASO/nK1WjlcoVqa4+QdtPam6ciW/bwdYCmk9ujS4Abe4VJ w2sueG/5d5jh11UgLmuXnMAdarDjSChSdXdKrhg3y7F8Pjptb4ff8WlByIdiSkfEbGFm PkRmC+09uBSPSsg344wCJdI8n7v1Yt+vr7kQ9DePitEy6ui34PJ0AU8C5jZKRsffEDfP ug3LM0AmYwwUZHhVNWlMxQzBz79rShyO1d1dkhvX1r4lmvgXBfWQm+lShZOQXq9rxarC SWsS9kfyfYspj/5U44fShRnUcSFmlbceeiOtALpaquQdvDHDNyVkshBPqlQYpF3P0oqm UHfw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Y4JE5dVY5pJ4L5p/4sGg1xocj3NSEFAKSZJJs4bfl50=; b=dt1Y0lNWiMP+OqLzllmpiE6aaw9GV3TVnmYzWnRapxrJaSYOjEw0aCvSqC/A3WGZRs yun6WWZtdelnXUWfBx9e4QGAubOO8iKKr+eBAStLfL4QkVK72ALQYeqanpzvxeeI8tA3 nBIoTOGT7nM+o0XP4yfEil9J0FermFkirIPCWlQqnyChLQl1bCHCs4Nkq7mNbAmLUS0J OggOdpgEGNXtAXsZ/x20dAYnm55s46XUXpVOtYxbk8+nbN6qWnPt6iXnSMqAmLhLgYnx O33aXO2DFozzSQPDehTWZZajyOk1ensWk+8nOz+QxBIj1GdHHiZi8tjU2Ri15mvooSM9 RFVg== X-Gm-Message-State: AOAM530CUNMzkHotZ4USQtf1VXqrNl6Aq1FMgCpbu4QadW6Dd6v3nWBf B6gZHzg/rteh8ErugsYektsvEjnh7UnweE+6b9ydTQ== X-Google-Smtp-Source: ABdhPJyyMkDOBVLKOsbECK6J7ZWww0BuYU20TDN6fYXnxk8xptFKTQXny9GfJRyE8hkAsArQFcu6jZmKOkIJSKODNlQ= X-Received: by 2002:a2e:98c3:0:b0:253:e0e1:20 with SMTP id s3-20020a2e98c3000000b00253e0e10020mr14949440ljj.26.1653517540546; Wed, 25 May 2022 15:25:40 -0700 (PDT) MIME-Version: 1.0 References: <20220523020209.11810-1-ojeda@kernel.org> <20220523020209.11810-22-ojeda@kernel.org> In-Reply-To: <20220523020209.11810-22-ojeda@kernel.org> From: Nick Desaulniers Date: Wed, 25 May 2022 15:25:28 -0700 Message-ID: Subject: Re: [PATCH v7 21/25] Kbuild: add Rust support To: Miguel Ojeda , Masahiro Yamada Cc: Linus Torvalds , Greg Kroah-Hartman , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Jarkko Sakkinen , Alex Gaynor , Finn Behrens , Adam Bratschi-Kaye , Wedson Almeida Filho , Michael Ellerman , Sven Van Asbroeck , Gary Guo , Boris-Chengbiao Zhou , Boqun Feng , Douglas Su , Dariusz Sosnowski , Antonio Terceiro , Daniel Xu , Miguel Cano , David Gow , Michal Marek , Russell King , Catalin Marinas , Will Deacon , Benjamin Herrenschmidt , Paul Mackerras , Paul Walmsley , Palmer Dabbelt , Albert Ou , Richard Weinberger , Anton Ivanov , Johannes Berg , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , linux-kbuild@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-um@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220525_152544_809143_7024D8EA X-CRM114-Status: GOOD ( 33.88 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, May 22, 2022 at 7:04 PM Miguel Ojeda wrote: > > --- a/Makefile > +++ b/Makefile > @@ -120,6 +120,15 @@ endif > > export KBUILD_CHECKSRC > > +# Enable "clippy" (a linter) as part of the Rust compilation. > +# > +# Use 'make CLIPPY=1' to enable it. > +ifeq ("$(origin CLIPPY)", "command line") > + KBUILD_CLIPPY := $(CLIPPY) > +endif Is there a reason to not just turn clippy on always? Might be nicer to start off with good practices by default. :^) > @@ -1494,7 +1588,8 @@ MRPROPER_FILES += include/config include/generated \ > certs/signing_key.pem \ > certs/x509.genkey \ > vmlinux-gdb.py \ > - *.spec > + *.spec \ > + rust/target.json rust/libmacros.so > > # clean - Delete most, but leave enough to build external modules > # > @@ -1519,6 +1614,9 @@ $(mrproper-dirs): > > mrproper: clean $(mrproper-dirs) > $(call cmd,rmfiles) > + @find . $(RCS_FIND_IGNORE) \ > + \( -name '*.rmeta' \) \ > + -type f -print | xargs rm -f Are there *.rmeta directories that we _don't_ want to remove via `make mrproper`? I'm curious why *.rmeta isn't just part of MRPROPER_FILES? > > # distclean > # > @@ -1606,6 +1704,23 @@ help: > @echo ' kselftest-merge - Merge all the config dependencies of' > @echo ' kselftest to existing .config.' > @echo '' > + @echo 'Rust targets:' > + @echo ' rustavailable - Checks whether the Rust toolchain is' > + @echo ' available and, if not, explains why.' > + @echo ' rustfmt - Reformat all the Rust code in the kernel' > + @echo ' rustfmtcheck - Checks if all the Rust code in the kernel' > + @echo ' is formatted, printing a diff otherwise.' > + @echo ' rustdoc - Generate Rust documentation' > + @echo ' (requires kernel .config)' > + @echo ' rusttest - Runs the Rust tests' > + @echo ' (requires kernel .config; downloads external repos)' > + @echo ' rust-analyzer - Generate rust-project.json rust-analyzer support file' > + @echo ' (requires kernel .config)' > + @echo ' dir/file.[os] - Build specified target only' > + @echo ' dir/file.i - Build macro expanded source, similar to C preprocessing' > + @echo ' (run with RUSTFMT=n to skip reformatting if needed)' > + @echo ' dir/file.ll - Build the LLVM assembly file' I don't think we need to repeat dir/* here again for rust. The existing targets listed above (outside this hunk) make sense in both contexts. Does rustc really use .i as a conventional suffix for macro expanded sources? (The C compiler might use the -x flag to override the guess it would make based on the file extension; I'm curious if rustc can ingest .i files or will it warn?) > diff --git a/init/Kconfig b/init/Kconfig > index ddcbefe535e9..3457cf596588 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > +config RUSTC_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(RUSTC) >/dev/null 2>&1 && $(RUSTC) --version || echo n) > + > +config BINDGEN_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(BINDGEN) >/dev/null 2>&1 && $(BINDGEN) --version || echo n) Are these two kconfigs used anywhere? > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > index 0496efd6e117..83e850321eb6 100644 > --- a/scripts/Kconfig.include > +++ b/scripts/Kconfig.include > @@ -36,12 +36,12 @@ ld-option = $(success,$(LD) -v $(1)) > as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -) > > # check if $(CC) and $(LD) exist > -$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found) > +$(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found) Not that it's important to do so, but a couple hunks s/compiler/C compiler/. Those _could_ probably get submitted ahead of this, but not important to do so. > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index 9f39b0130551..fe87389d52c0 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -1,4 +1,5 @@ > DEBUG_CFLAGS := > +DEBUG_RUSTFLAGS := > > ifdef CONFIG_DEBUG_INFO_SPLIT > DEBUG_CFLAGS += -gsplit-dwarf > @@ -10,6 +11,12 @@ ifndef CONFIG_AS_IS_LLVM > KBUILD_AFLAGS += -Wa,-gdwarf-2 > endif > > +ifdef CONFIG_DEBUG_INFO_REDUCED > +DEBUG_RUSTFLAGS += -Cdebuginfo=1 > +else > +DEBUG_RUSTFLAGS += -Cdebuginfo=2 > +endif > + How does enabling or disabling debug info work for rustc? I may have missed it, but I was surprised to see no additional flags getting passed to rustc based on CONFIG_DEBUG info. > diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs > new file mode 100644 > index 000000000000..c146a3407183 > --- /dev/null > +++ b/scripts/generate_rust_target.rs Ah, that explains the host rust build infra. Bravo! Hard coding the target files was my major concern last I looked at the series. I'm very happy to see it be generated properly from .config! I haven't actually reviewed this yet, but it makes me significantly more confident in the series to see this approach added. Good job whoever wrote this. > diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh > new file mode 100755 > index 000000000000..277a64d07f22 > --- /dev/null > +++ b/scripts/is_rust_module.sh > @@ -0,0 +1,13 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# is_rust_module.sh module.ko > +# > +# Returns `0` if `module.ko` is a Rust module, `1` otherwise. > + > +set -e > + > +# Using the `16_` prefix ensures other symbols with the same substring > +# are not picked up (even if it would be unlikely). The last part is > +# used just in case LLVM decides to use the `.` suffix. > +${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$' Does `$(READELF) -p .comment foo.o` print anything about which compiler was used? That seems less brittle IMO. --- Modulo the RUST_OPT_LEVEL_SIMILAR_AS_CHOSEN_FOR_C which I'm not a fan of, this is looking good to me. Masahiro, what are your thoughts on the build system support? -- Thanks, ~Nick Desaulniers _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-x232.google.com ([2a00:1450:4864:20::232]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ntzRo-00Crs4-NN for linux-um@lists.infradead.org; Wed, 25 May 2022 22:25:46 +0000 Received: by mail-lj1-x232.google.com with SMTP id r3so19118021ljd.7 for ; Wed, 25 May 2022 15:25:42 -0700 (PDT) MIME-Version: 1.0 References: <20220523020209.11810-1-ojeda@kernel.org> <20220523020209.11810-22-ojeda@kernel.org> In-Reply-To: <20220523020209.11810-22-ojeda@kernel.org> From: Nick Desaulniers Date: Wed, 25 May 2022 15:25:28 -0700 Message-ID: Subject: Re: [PATCH v7 21/25] Kbuild: add Rust support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: Miguel Ojeda , Masahiro Yamada Cc: Linus Torvalds , Greg Kroah-Hartman , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Jarkko Sakkinen , Alex Gaynor , Finn Behrens , Adam Bratschi-Kaye , Wedson Almeida Filho , Michael Ellerman , Sven Van Asbroeck , Gary Guo , Boris-Chengbiao Zhou , Boqun Feng , Douglas Su , Dariusz Sosnowski , Antonio Terceiro , Daniel Xu , Miguel Cano , David Gow , Michal Marek , Russell King , Catalin Marinas , Will Deacon , Benjamin Herrenschmidt , Paul Mackerras , Paul Walmsley , Palmer Dabbelt , Albert Ou , Richard Weinberger , Anton Ivanov , Johannes Berg , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , linux-kbuild@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-um@lists.infradead.org On Sun, May 22, 2022 at 7:04 PM Miguel Ojeda wrote: > > --- a/Makefile > +++ b/Makefile > @@ -120,6 +120,15 @@ endif > > export KBUILD_CHECKSRC > > +# Enable "clippy" (a linter) as part of the Rust compilation. > +# > +# Use 'make CLIPPY=1' to enable it. > +ifeq ("$(origin CLIPPY)", "command line") > + KBUILD_CLIPPY := $(CLIPPY) > +endif Is there a reason to not just turn clippy on always? Might be nicer to start off with good practices by default. :^) > @@ -1494,7 +1588,8 @@ MRPROPER_FILES += include/config include/generated \ > certs/signing_key.pem \ > certs/x509.genkey \ > vmlinux-gdb.py \ > - *.spec > + *.spec \ > + rust/target.json rust/libmacros.so > > # clean - Delete most, but leave enough to build external modules > # > @@ -1519,6 +1614,9 @@ $(mrproper-dirs): > > mrproper: clean $(mrproper-dirs) > $(call cmd,rmfiles) > + @find . $(RCS_FIND_IGNORE) \ > + \( -name '*.rmeta' \) \ > + -type f -print | xargs rm -f Are there *.rmeta directories that we _don't_ want to remove via `make mrproper`? I'm curious why *.rmeta isn't just part of MRPROPER_FILES? > > # distclean > # > @@ -1606,6 +1704,23 @@ help: > @echo ' kselftest-merge - Merge all the config dependencies of' > @echo ' kselftest to existing .config.' > @echo '' > + @echo 'Rust targets:' > + @echo ' rustavailable - Checks whether the Rust toolchain is' > + @echo ' available and, if not, explains why.' > + @echo ' rustfmt - Reformat all the Rust code in the kernel' > + @echo ' rustfmtcheck - Checks if all the Rust code in the kernel' > + @echo ' is formatted, printing a diff otherwise.' > + @echo ' rustdoc - Generate Rust documentation' > + @echo ' (requires kernel .config)' > + @echo ' rusttest - Runs the Rust tests' > + @echo ' (requires kernel .config; downloads external repos)' > + @echo ' rust-analyzer - Generate rust-project.json rust-analyzer support file' > + @echo ' (requires kernel .config)' > + @echo ' dir/file.[os] - Build specified target only' > + @echo ' dir/file.i - Build macro expanded source, similar to C preprocessing' > + @echo ' (run with RUSTFMT=n to skip reformatting if needed)' > + @echo ' dir/file.ll - Build the LLVM assembly file' I don't think we need to repeat dir/* here again for rust. The existing targets listed above (outside this hunk) make sense in both contexts. Does rustc really use .i as a conventional suffix for macro expanded sources? (The C compiler might use the -x flag to override the guess it would make based on the file extension; I'm curious if rustc can ingest .i files or will it warn?) > diff --git a/init/Kconfig b/init/Kconfig > index ddcbefe535e9..3457cf596588 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > +config RUSTC_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(RUSTC) >/dev/null 2>&1 && $(RUSTC) --version || echo n) > + > +config BINDGEN_VERSION_TEXT > + string > + depends on RUST > + default $(shell,command -v $(BINDGEN) >/dev/null 2>&1 && $(BINDGEN) --version || echo n) Are these two kconfigs used anywhere? > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include > index 0496efd6e117..83e850321eb6 100644 > --- a/scripts/Kconfig.include > +++ b/scripts/Kconfig.include > @@ -36,12 +36,12 @@ ld-option = $(success,$(LD) -v $(1)) > as-instr = $(success,printf "%b\n" "$(1)" | $(CC) $(CLANG_FLAGS) -c -x assembler -o /dev/null -) > > # check if $(CC) and $(LD) exist > -$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found) > +$(error-if,$(failure,command -v $(CC)),C compiler '$(CC)' not found) Not that it's important to do so, but a couple hunks s/compiler/C compiler/. Those _could_ probably get submitted ahead of this, but not important to do so. > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug > index 9f39b0130551..fe87389d52c0 100644 > --- a/scripts/Makefile.debug > +++ b/scripts/Makefile.debug > @@ -1,4 +1,5 @@ > DEBUG_CFLAGS := > +DEBUG_RUSTFLAGS := > > ifdef CONFIG_DEBUG_INFO_SPLIT > DEBUG_CFLAGS += -gsplit-dwarf > @@ -10,6 +11,12 @@ ifndef CONFIG_AS_IS_LLVM > KBUILD_AFLAGS += -Wa,-gdwarf-2 > endif > > +ifdef CONFIG_DEBUG_INFO_REDUCED > +DEBUG_RUSTFLAGS += -Cdebuginfo=1 > +else > +DEBUG_RUSTFLAGS += -Cdebuginfo=2 > +endif > + How does enabling or disabling debug info work for rustc? I may have missed it, but I was surprised to see no additional flags getting passed to rustc based on CONFIG_DEBUG info. > diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs > new file mode 100644 > index 000000000000..c146a3407183 > --- /dev/null > +++ b/scripts/generate_rust_target.rs Ah, that explains the host rust build infra. Bravo! Hard coding the target files was my major concern last I looked at the series. I'm very happy to see it be generated properly from .config! I haven't actually reviewed this yet, but it makes me significantly more confident in the series to see this approach added. Good job whoever wrote this. > diff --git a/scripts/is_rust_module.sh b/scripts/is_rust_module.sh > new file mode 100755 > index 000000000000..277a64d07f22 > --- /dev/null > +++ b/scripts/is_rust_module.sh > @@ -0,0 +1,13 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# is_rust_module.sh module.ko > +# > +# Returns `0` if `module.ko` is a Rust module, `1` otherwise. > + > +set -e > + > +# Using the `16_` prefix ensures other symbols with the same substring > +# are not picked up (even if it would be unlikely). The last part is > +# used just in case LLVM decides to use the `.` suffix. > +${NM} "$*" | grep -qE '^[0-9a-fA-F]+ r _R[^[:space:]]+16___IS_RUST_MODULE[^[:space:]]*$' Does `$(READELF) -p .comment foo.o` print anything about which compiler was used? That seems less brittle IMO. --- Modulo the RUST_OPT_LEVEL_SIMILAR_AS_CHOSEN_FOR_C which I'm not a fan of, this is looking good to me. Masahiro, what are your thoughts on the build system support? -- Thanks, ~Nick Desaulniers _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um