After the authorship debacle on the RFC, I've tried to be even more
careful this time around. Gary opted for a Co-developed-by in the replies
of the RFC stuff, so I have given them one.
I have added SoB's too, but if that is not okay Gary, then please scream
loudly.
As this is lifted from the state of the Rust-for-Linux tree, the commit
messages from there cannot be preserved, so these patches have commit
messages that I wrote.
I've tested this on Icicle, and the modules seem to work as expected.
Unfortunately there appear to be implicit 32-bit divisions (or similar)
in core Rust code, so, as in the downstream Rust-for-Linux tree, Rust is
only enabled for 64-bit.
Thanks,
Conor.
Changes in v1:
- rebase on v6.3-rc1
- resort the `um` entry in the arch-support table while adding RISC-V
to it
- drop 32-bit bits
- have another crack at assigning authorship
Changes in RFC-RESEND:
- fix the asymmetrical additions in the Makefile bits
- add cc-cover to my git send-email command...
CC: Miguel Ojeda <[email protected]>
CC: Alex Gaynor <[email protected]>
CC: Wedson Almeida Filho <[email protected]>
CC: Boqun Feng <[email protected]>
CC: Gary Guo <[email protected]>
CC: Björn Roy Baron <[email protected]>
CC: Jonathan Corbet <[email protected]>
CC: Paul Walmsley <[email protected]>
CC: Palmer Dabbelt <[email protected]>
CC: Nathan Chancellor <[email protected]>
CC: Nick Desaulniers <[email protected]>
CC: Tom Rix <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
Miguel Ojeda (2):
scripts: generate_rust_target: enable building on RISC-V
RISC-V: enable building 64-bit kernels with rust support
Documentation/rust/arch-support.rst | 3 ++-
arch/riscv/Kconfig | 1 +
arch/riscv/Makefile | 2 ++
scripts/generate_rust_target.rs | 16 ++++++++++++++++
4 files changed, 21 insertions(+), 1 deletion(-)
--
2.39.2
From: Miguel Ojeda <[email protected]>
Add the required bits from rust-for-linux to enable generating a RISC-V
target for rust. The script, written by Miguel, was originally a
config file contributed by Gary.
Co-developed-by: Gary Guo <[email protected]>
Signed-off-by: Gary Guo <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Despite removing 32-bit support, I kept the structure of the if
statement, despite early return being stylistically preferred, for
alignment with the Rust-for-Linux tree. I'm happy to respin to sort that
out of desired.
---
scripts/generate_rust_target.rs | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs
index 3c6cbe2b278d3..85d690f764389 100644
--- a/scripts/generate_rust_target.rs
+++ b/scripts/generate_rust_target.rs
@@ -161,6 +161,22 @@ fn main() {
ts.push("features", features);
ts.push("llvm-target", "x86_64-linux-gnu");
ts.push("target-pointer-width", "64");
+ } else if cfg.has("RISCV") {
+ if cfg.has("64BIT") {
+ ts.push("arch", "riscv64");
+ ts.push("data-layout", "e-m:e-p:64:64-i64:64-i128:128-n64-S128");
+ ts.push("llvm-target", "riscv64-linux-gnu");
+ ts.push("target-pointer-width", "64");
+ } else {
+ panic!("32-bit RISC-V is an unsupported architecture")
+ }
+ ts.push("code-model", "medium");
+ ts.push("disable-redzone", true);
+ let mut features = "+m,+a".to_string();
+ if cfg.has("RISCV_ISA_C") {
+ features += ",+c";
+ }
+ ts.push("features", features);
} else {
panic!("Unsupported architecture");
}
--
2.39.2
From: Miguel Ojeda <[email protected]>
The rust modules work on 64-bit RISC-V, with no twiddling required.
Select HAVE_RUST and provide the required flags to kbuild so that the
modules can be used. The Makefile and Kconfig changes are lifted from
work done by Miguel in the Rust-for-Linux tree, hence his authorship.
Following the rabbit hole, the Makefile changes originated in a script,
created based on config files originally added by Gary, hence his
co-authorship.
32-bit is broken in core rust code, so support is limited to 64-bit:
ld.lld: error: undefined symbol: __udivdi3
As 64-bit RISC-V is now supported, add it to the arch support table,
taking the opportunity to sort the table in alphabetical order.
Co-developed-by: Gary Guo <[email protected]>
Signed-off-by: Gary Guo <[email protected]>
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
While adding RISC-V to the table, I took the chance to re-sort it
alphabetically. I didn't think that warranted co-authorship, but given
we had an authorship conversation already I am happy to provide one for
that...
---
Documentation/rust/arch-support.rst | 3 ++-
arch/riscv/Kconfig | 1 +
arch/riscv/Makefile | 2 ++
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/rust/arch-support.rst b/Documentation/rust/arch-support.rst
index ed7f4f5b3cf15..77765ffd5af41 100644
--- a/Documentation/rust/arch-support.rst
+++ b/Documentation/rust/arch-support.rst
@@ -15,7 +15,8 @@ support corresponds to ``S`` values in the ``MAINTAINERS`` file.
============ ================ ==============================================
Architecture Level of support Constraints
============ ================ ==============================================
-``x86`` Maintained ``x86_64`` only.
+``riscv`` Maintained ``riscv64`` only.
``um`` Maintained ``x86_64`` only.
+``x86`` Maintained ``x86_64`` only.
============ ================ ==============================================
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index c5e42cc376048..c3179b139361f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -114,6 +114,7 @@ config RISCV
select HAVE_POSIX_CPU_TIMERS_TASK_WORK
select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_RSEQ
+ select HAVE_RUST if 64BIT
select HAVE_STACKPROTECTOR
select HAVE_SYSCALL_TRACEPOINTS
select IRQ_DOMAIN
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 6203c33789228..950612bf193cf 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -31,6 +31,8 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
KBUILD_AFLAGS += -mabi=lp64
KBUILD_LDFLAGS += -melf64lriscv
+
+ KBUILD_RUSTFLAGS += -Ctarget-cpu=generic-rv64
else
BITS := 32
UTS_MACHINE := riscv32
--
2.39.2
On Tue, Mar 7, 2023 at 11:25 AM Conor Dooley <[email protected]> wrote:
>
> While adding RISC-V to the table, I took the chance to re-sort it
> alphabetically.
For reference, there is a patch for this coming at:
https://lore.kernel.org/rust-for-linux/I0YeaNjTtc4Nh47ZLJfAs6rgfAc_QZxhynNfz-GQKssVZ1S2UI_cTScCkp9-oX-hPYVcP3EfF7N0HMB9iAlm1FcvOJagnQoLeHtiW3bGCgM=@bamelis.dev/
Cheers,
Miguel
On Tue, Mar 07, 2023 at 11:56:30AM +0100, Miguel Ojeda wrote:
> On Tue, Mar 7, 2023 at 11:25 AM Conor Dooley <[email protected]> wrote:
> >
> > While adding RISC-V to the table, I took the chance to re-sort it
> > alphabetically.
>
> For reference, there is a patch for this coming at:
>
> https://lore.kernel.org/rust-for-linux/I0YeaNjTtc4Nh47ZLJfAs6rgfAc_QZxhynNfz-GQKssVZ1S2UI_cTScCkp9-oX-hPYVcP3EfF7N0HMB9iAlm1FcvOJagnQoLeHtiW3bGCgM=@bamelis.dev/
Cool. Git should resolve that, probably without even generating a
conflict in -next, right?
On Tue, Mar 7, 2023 at 11:25 AM Conor Dooley <[email protected]> wrote:
>
> I have added SoB's too, but if that is not okay Gary, then please scream
> loudly.
Note that `Co-developed-by`s always go with a `Signed-off-by`s, i.e.
it is not possible to add just a `Co-developed-by`.
By the way, like for the Arm patch set, if you end up doing a v2,
could you please add the `BINDGEN_TARGET_*` in `rust/Makefile` (GCC
builds are really experimental, but since they are there anyway, it is
best to be consistent and add it).
Cheers,
Miguel
On Tue, Mar 7, 2023 at 11:25 AM Conor Dooley <[email protected]> wrote:
>
> Despite removing 32-bit support, I kept the structure of the if
> statement, despite early return being stylistically preferred, for
> alignment with the Rust-for-Linux tree. I'm happy to respin to sort that
> out of desired.
This is a case of 2 "equal" sides to the branch (though at the moment
an error), so it sounds good, and it will mean a smaller diff later.
> + panic!("32-bit RISC-V is an unsupported architecture")
Nit: if there is a v2, please add a semicolon to be consistent with
the others in the file (not sure which style we will go for, it looks
like `rustfmt` accepts both ways).
Cheers,
Miguel
On Tue, Mar 7, 2023 at 12:01 PM Conor Dooley <[email protected]> wrote:
>
> Cool. Git should resolve that, probably without even generating a
> conflict in -next, right?
I think it will conflict.
Since the cleanup is elsewhere and it may add work for the -next
maintainer, it is probably best to take it out in v2.
Cheers,
Miguel
On Tue, Mar 07, 2023 at 12:56:30PM +0100, Miguel Ojeda wrote:
> On Tue, Mar 7, 2023 at 12:01 PM Conor Dooley <[email protected]> wrote:
> >
> > Cool. Git should resolve that, probably without even generating a
> > conflict in -next, right?
>
> I think it will conflict.
>
> Since the cleanup is elsewhere and it may add work for the -next
> maintainer, it is probably best to take it out in v2.
If you think it will conflict as-is, it's probably gonna conflict either
way.
You've pointed out several small bits, I'll send a v2 in a few days.
Cheers,
Conor.
On Tue, Mar 07, 2023 at 12:07:29PM +0100, Miguel Ojeda wrote:
> On Tue, Mar 7, 2023 at 11:25 AM Conor Dooley <[email protected]> wrote:
> >
> > I have added SoB's too, but if that is not okay Gary, then please scream
> > loudly.
>
> Note that `Co-developed-by`s always go with a `Signed-off-by`s, i.e.
> it is not possible to add just a `Co-developed-by`.
Aye, but that does not mean that I am entitled to add someone else's!
> By the way, like for the Arm patch set, if you end up doing a v2,
> could you please add the `BINDGEN_TARGET_*` in `rust/Makefile` (GCC
> builds are really experimental, but since they are there anyway, it is
> best to be consistent and add it).
Sure.
On Thu, Mar 30, 2023 at 09:23:45AM +0100, Conor Dooley wrote:
> On Tue, Mar 07, 2023 at 12:07:29PM +0100, Miguel Ojeda wrote:
> > By the way, like for the Arm patch set, if you end up doing a v2,
> > could you please add the `BINDGEN_TARGET_*` in `rust/Makefile` (GCC
> > builds are really experimental, but since they are there anyway, it is
> > best to be consistent and add it).
Hmm, so I came across this commit while looking at that:
https://github.com/Rust-for-Linux/linux/commit/cfc17fed52b9585e2f19e2381bfb7094561b8027a
(rust: bindgen: ignore RISC-V extensions for GCC builds)
I don't want to add even more workarounds for this sort of thing,
especially as in this case it is outside of arch/riscv.
The extension stuff when mixing compilers is such a massive pain & given
this one requires Rust it's even less likely to be tested when someone
comes along and adds some additional extension support that appears in
-march :(
I'd rather do this in the RISC-V Makefile so that it does not get
forgotten.
If my understanding of bindgen is correct, we don't actually need to be
honest to it about what extensions the rest of the kernel is compiled
with, only make sure that it is not called with arguments it does not
understand?
| bindgen_c_flags_patsubst1 = $(patsubst -march=rv%_zicbom,-march=rv%,$(bindgen_c_flags_patsubst2))
This one is no longer needed as of 9a5c09dd9701 ("Merge patch series
"Remove toolchain dependencies for Zicbom"").
| bindgen_c_flags_patsubst = $(patsubst -march=rv%_zicsr_zifencei,-march=rv%,$(bindgen_c_flags_patsubst1))
Oh and clang-17 is going to support both of these, and Nathan and I
already spent a bunch of time fixing the fallout from that!
It still functions correctly without having them passed, but I have
heard requiring these may become the default at some point too.
What's done here may end up needing to be dynamic, but that bridge can be
crossed if/when we come to it.
What version of GCC do I need to replicate this? I can build tip-of-tree
gcc if needs be.
Cheers,
Conor.
On Thu, Mar 30, 2023 at 10:24 AM Conor Dooley
<[email protected]> wrote:
>
> Aye, but that does not mean that I am entitled to add someone else's!
Yeah, definitely! I meant that, from what you said, it sounded like
adding the `Co-developed-by` was OK without the other (i.e. due to the
"too" in your sentence).
Cheers,
Miguel
On Thu, Mar 30, 2023 at 11:12 AM Conor Dooley
<[email protected]> wrote:
>
> I'd rather do this in the RISC-V Makefile so that it does not get
> forgotten.
Sounds good to me! We want to have the least amount of things possible
in the common pieces (e.g. for the target spec file we moved some
flags); so the more we move out to `arch/`, the better.
> If my understanding of bindgen is correct, we don't actually need to be
> honest to it about what extensions the rest of the kernel is compiled
> with, only make sure that it is not called with arguments it does not
> understand?
As long as bindgen generates things with the right ABI etc., yeah.
But, in principle, enabling one extension one side but not the other
could be wrong if it ends up in something that Rust uses, e.g. if the
C side does:
#ifdef __ARM_ARCH_7R__
int x;
#else
char x;
#endif
and Rust attempts to use it, then particular `-march` builds could be broken.
> Oh and clang-17 is going to support both of these, and Nathan and I
> already spent a bunch of time fixing the fallout from that!
> It still functions correctly without having them passed, but I have
> heard requiring these may become the default at some point too.
> What's done here may end up needing to be dynamic, but that bridge can be
> crossed if/when we come to it.
>
> What version of GCC do I need to replicate this? I can build tip-of-tree
> gcc if needs be.
Sorry, what do you want to replicate? If you mean what we had in the
old GitHub CI, I see:
CONFIG_CC_VERSION_TEXT="riscv64-linux-gnu-gcc (Ubuntu
11.3.0-1ubuntu1~22.04) 11.3.0"
which successfully boots in QEMU for the kernel config we tested.
But if you are asking what should be supported, I guess it depends on
the RISC-V maintainers. Ideally, everything that the kernel supports
(GCC >= 5.1), but since the GCC+Rust builds are so experimental, I
think as long as something is tested from time to time, it would be
great (to at least know not everything is completely broken).
But if you think that would be too much effort to maintain, or even
GCC builds in general, then please feel free to ignore it for the time
being, i.e. it is better to have LLVM builds rather than nothing! :)
Cheers,
Miguel
On Mon, Apr 03, 2023 at 06:35:45PM +0200, Miguel Ojeda wrote:
> On Thu, Mar 30, 2023 at 11:12 AM Conor Dooley
> <[email protected]> wrote:
> >
> > I'd rather do this in the RISC-V Makefile so that it does not get
> > forgotten.
>
> Sounds good to me! We want to have the least amount of things possible
> in the common pieces (e.g. for the target spec file we moved some
> flags); so the more we move out to `arch/`, the better.
>
> > If my understanding of bindgen is correct, we don't actually need to be
> > honest to it about what extensions the rest of the kernel is compiled
> > with, only make sure that it is not called with arguments it does not
> > understand?
>
> As long as bindgen generates things with the right ABI etc., yeah.
> But, in principle, enabling one extension one side but not the other
> could be wrong if it ends up in something that Rust uses, e.g. if the
> C side does:
>
> #ifdef __ARM_ARCH_7R__
> int x;
> #else
> char x;
> #endif
>
> and Rust attempts to use it, then particular `-march` builds could be broken.
To be on the safe side then, we should really disable the extensions
across the whole kernel. I don't *think* we have any madness at the
moment like in the above, but it is better to be on the safe side.
As I note below, it's just one extension for now anyway.
> > What version of GCC do I need to replicate this? I can build tip-of-tree
> > gcc if needs be.
>
> Sorry, what do you want to replicate? If you mean what we had in the
> old GitHub CI, I see:
>
> CONFIG_CC_VERSION_TEXT="riscv64-linux-gnu-gcc (Ubuntu
> 11.3.0-1ubuntu1~22.04) 11.3.0"
>
> which successfully boots in QEMU for the kernel config we tested.
No, I misunderstood your question. I thought you meant something else
entirely.
> But if you are asking what should be supported, I guess it depends on
> the RISC-V maintainers. Ideally, everything that the kernel supports
> (GCC >= 5.1),
Heh, as if that number is true across the board!
> but since the GCC+Rust builds are so experimental, I
> think as long as something is tested from time to time, it would be
> great (to at least know not everything is completely broken).
>
> But if you think that would be too much effort to maintain, or even
> GCC builds in general, then please feel free to ignore it for the time
> being, i.e. it is better to have LLVM builds rather than nothing! :)
Yeah, it may be worth getting just the LLVM bits in. I abhor the -march
handling and it may end up looking like shite with the zicsr &
zifencei handling.
Worst comes to worst, can permit gcc builds by just removing all the
extensions that get passed in -march for RUST && CC_IS_GCC type
scenarios. The only one of those at the moment is zihintpause & I don't
suppose too many tears will be shed over that.
For now it's safe to assume that LLVM doesn't require zicsr or zifencei
[1], we don't need to do a version dance right away.
¯\_(ツ)_/¯,
Conor.
1 - https://reviews.llvm.org/D147183#4233360
On Mon, Apr 03, 2023 at 06:14:57PM +0100, Conor Dooley wrote:
> On Mon, Apr 03, 2023 at 06:35:45PM +0200, Miguel Ojeda wrote:
> > As long as bindgen generates things with the right ABI etc., yeah.
> > But, in principle, enabling one extension one side but not the other
> > could be wrong if it ends up in something that Rust uses, e.g. if the
> > C side does:
> >
> > #ifdef __ARM_ARCH_7R__
> > int x;
> > #else
> > char x;
> > #endif
> >
> > and Rust attempts to use it, then particular `-march` builds could be broken.
>
> To be on the safe side then, we should really disable the extensions
> across the whole kernel. I don't *think* we have any madness at the
> moment like in the above, but it is better to be on the safe side.
So I am still of this opinion. I don't want to silently have a mismatch
between one side of the kernel and the other. Recipe for disaster.
If it's off for the Rust side of things, it should be off for C too.
> > but since the GCC+Rust builds are so experimental, I
> > think as long as something is tested from time to time, it would be
> > great (to at least know not everything is completely broken).
> >
> > But if you think that would be too much effort to maintain, or even
> > GCC builds in general, then please feel free to ignore it for the time
> > being, i.e. it is better to have LLVM builds rather than nothing! :)
>
> Yeah, it may be worth getting just the LLVM bits in. I abhor the -march
> handling and it may end up looking like shite with the zicsr &
> zifencei handling.
> Worst comes to worst, can permit gcc builds by just removing all the
> extensions that get passed in -march for RUST && CC_IS_GCC type
> scenarios. The only one of those at the moment is zihintpause & I don't
> suppose too many tears will be shed over that.
Been thinking about this some more, and I don't really like where this
is going. I think I am gonna explicitly disable gcc support if
anything.
I wrote out a list of issues I have with all of this, but I then had
second thoughts about some of them, so I've deleted that section of this
mail.
I need to think long and hard about the mixing and matching of support
between several versions of the tools (bindgen/llvm, rustc, gcc) for
different extensions & potentially different versions of the ISA spec.
I'll revisit this when my thoughts have settled down.
> For now it's safe to assume that LLVM doesn't require zicsr or zifencei
> [1], we don't need to do a version dance right away.
I also needed to remove `-mno-riscv-attribute` from bindgen's cflags
for things to work. That's probably not something yous have to deal with
as you're on an old kernel for the rust branch. Or maybe it got
backported to v6.2.n, idk.
Oh and bindgen doesn't actually seem to succeed with the hacks anyway:
thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/__/include/linux/compiler_types_h_146_2)" is not a valid Ident'
I had a quick check on lore but didn't see a fix for that one.
And there's also the code model that doesn't yet seem to be handled.
The script looks to always use medany. Writing that here lest I forget
about it.
Either way, I marked the series as "Changes Requested" on patchwork :)
Cheers,
Conor.
Whoops, actually to Kwang this time...
On Thu, Jun 08, 2023 at 08:01:16AM +0100, Conor Dooley wrote:
> Hey Kwang,
>
> On 08/06/2023 05:46, 손광훈/Tizen Platform Lab(SR)/삼성전자 wrote:
> > Hi,
> > Recently I'm trying to put a rust patch on the risc-v board.
> > I saw a patch [1] and looked through it roughly.
> > Only if llvm(not gcc) is allowed, it looks good with no major problems.
> >
> > > I'll revisit this when my thoughts have settled down.
> >
> > If you let me know the problematic part, may I try the patch?
> >
> > [1] https://lore.kernel.org/linux-riscv/20230405-itinerary-handgrip-
> > a5ffba368148@spud/
>
> Yeah, you can definitely try this or the downstream rust-for-linux
> project - both should work well on RISC-V.
> The problematic part is figuring out what ISA extensions are supported
> by the rust compiler being used (and by bindgen), and deciding what to
> put in -march as a result.
>
> I think it is unlikely to matter for you, unless you're aggressively
> mixing versions for different parts of your toolchain.
>
> I do intend revisting this, probably after the min. version for rust
> gets bumped, I've just been really busy with other work the last weeks.
>
> Cheers,
> Conor.
Hey Kwang,
On 08/06/2023 05:46, 손광훈/Tizen Platform Lab(SR)/삼성전자 wrote:
> Hi,
> Recently I'm trying to put a rust patch on the risc-v board.
> I saw a patch [1] and looked through it roughly.
> Only if llvm(not gcc) is allowed, it looks good with no major problems.
>
> > I'll revisit this when my thoughts have settled down.
>
> If you let me know the problematic part, may I try the patch?
>
> [1] https://lore.kernel.org/linux-riscv/20230405-itinerary-handgrip-
> a5ffba368148@spud/
Yeah, you can definitely try this or the downstream rust-for-linux
project - both should work well on RISC-V.
The problematic part is figuring out what ISA extensions are supported
by the rust compiler being used (and by bindgen), and deciding what to
put in -march as a result.
I think it is unlikely to matter for you, unless you're aggressively
mixing versions for different parts of your toolchain.
I do intend revisting this, probably after the min. version for rust
gets bumped, I've just been really busy with other work the last weeks.
Cheers,
Conor.
> Hey Kwang,
>
> > Hi,
> > Recently I'm trying to put a rust patch on the risc-v board.
> > I saw a patch [1] and looked through it roughly.
> > Only if llvm(not gcc) is allowed, it looks good with no major problems.
> >
> > > I'll revisit this when my thoughts have settled down.
> >
> > If you let me know the problematic part, may I try the patch?
> >
> > [1] https://lore.kernel.org/linux-riscv/20230405-itinerary-handgrip-
> > a5ffba368148@spud/
>
> Yeah, you can definitely try this or the downstream rust-for-linux
> project - both should work well on RISC-V.
> The problematic part is figuring out what ISA extensions are supported
> by the rust compiler being used (and by bindgen), and deciding what to
> put in -march as a result.
>
> I think it is unlikely to matter for you, unless you're aggressively
> mixing versions for different parts of your toolchain.
>
> I do intend revisting this, probably after the min. version for rust
> gets bumped, I've just been really busy with other work the last weeks.
No rush! I was just curious.
Thank you for the explanation!
>
> Cheers,
> Conor.
On Thu, Jun 8, 2023 at 9:01 AM Conor Dooley <[email protected]> wrote:
>
> I do intend revisting this, probably after the min. version for rust
> gets bumped, I've just been really busy with other work the last weeks.
Thanks Conor! That would be great. We are increasing the minimum
version after the merge window to Rust 1.70.0 (assuming no unexpected
issues).
This is the branch I have in case you want to use it, I will submit it
soon: https://github.com/ojeda/linux/tree/rust-1.70
Cheers,
Miguel
On Thu, Jun 08, 2023 at 01:52:47PM +0200, Miguel Ojeda wrote:
> On Thu, Jun 8, 2023 at 9:01 AM Conor Dooley <[email protected]> wrote:
> >
> > I do intend revisting this, probably after the min. version for rust
> > gets bumped, I've just been really busy with other work the last weeks.
>
> Thanks Conor! That would be great. We are increasing the minimum
> version after the merge window to Rust 1.70.0 (assuming no unexpected
> issues).
Right, so probably I won't resubmit anything until after v6.6 then,
as it won't be in the RISC-V tree until then, by the sounds of your
timeline.
Gives me plenty of time to try and unravel the mess of libclang versions
and what extensions are supported by each tool. Not like I am devoid of
other things that need to be done!
Palmer, Miguel, Nathan, etc
On Thu, Jun 08, 2023 at 01:28:06PM +0100, Conor Dooley wrote:
> On Thu, Jun 08, 2023 at 01:52:47PM +0200, Miguel Ojeda wrote:
> > On Thu, Jun 8, 2023 at 9:01 AM Conor Dooley <[email protected]> wrote:
> > >
> > > I do intend revisting this, probably after the min. version for rust
> > > gets bumped, I've just been really busy with other work the last weeks.
> >
> > Thanks Conor! That would be great. We are increasing the minimum
> > version after the merge window to Rust 1.70.0 (assuming no unexpected
> > issues).
>
> Right, so probably I won't resubmit anything until after v6.6 then,
> as it won't be in the RISC-V tree until then, by the sounds of your
> timeline.
> Gives me plenty of time to try and unravel the mess of libclang versions
> and what extensions are supported by each tool. Not like I am devoid of
> other things that need to be done!
6.6 came and went, and I have been busy dealing with the other
responsibilities I mentioned and have not had a chance to look here.
I rebased this today and things still work as they did when I submitted
this version, but things have gotten muddier on the LLVM side of things,
as more recent versions have added yet more extension support.
My inclination at this point is to engage in a bit of LARPing as an
ostrich, and sorta ignore these concerns initially. Specifically, I'd
like to drop the idea of having the gcc support, and restrict to LLVM=1.
When it comes to asymmetrical extension support between the C and Rust
toolchains, I'm think we deal with that as we do for the C toolchains,
sort issues out as-and-when they arrive rather than punt this again.
Thoughts?
On Wed, Jan 17, 2024 at 12:31 PM Conor Dooley <[email protected]> wrote:
>
> 6.6 came and went, and I have been busy dealing with the other
> responsibilities I mentioned and have not had a chance to look here.
> I rebased this today and things still work as they did when I submitted
> this version, but things have gotten muddier on the LLVM side of things,
> as more recent versions have added yet more extension support.
Sounds fun :)
> My inclination at this point is to engage in a bit of LARPing as an
> ostrich, and sorta ignore these concerns initially. Specifically, I'd
> like to drop the idea of having the gcc support, and restrict to LLVM=1.
Yeah, if `LLVM=1` works, then I would suggest going ahead with that.
(Now that `rustc_codegen_gcc` is here, we will move to that and forget
about mixed compiler builds, but we still have to handle `bindgen`
flags until we have an alternative for that)
> When it comes to asymmetrical extension support between the C and Rust
> toolchains, I'm think we deal with that as we do for the C toolchains,
> sort issues out as-and-when they arrive rather than punt this again.
Sounds good, thanks a lot!
Cheers,
Miguel
On Wed, Jan 17, 2024 at 07:23:17PM +0100, Miguel Ojeda wrote:
> On Wed, Jan 17, 2024 at 12:31 PM Conor Dooley <[email protected]> wrote:
> >
> > 6.6 came and went, and I have been busy dealing with the other
> > responsibilities I mentioned and have not had a chance to look here.
> > I rebased this today and things still work as they did when I submitted
> > this version, but things have gotten muddier on the LLVM side of things,
> > as more recent versions have added yet more extension support.
>
> Sounds fun :)
>
> > My inclination at this point is to engage in a bit of LARPing as an
> > ostrich, and sorta ignore these concerns initially. Specifically, I'd
> > like to drop the idea of having the gcc support, and restrict to LLVM=1.
>
> Yeah, if `LLVM=1` works, then I would suggest going ahead with that.
>
> (Now that `rustc_codegen_gcc` is here, we will move to that and forget
> about mixed compiler builds, but we still have to handle `bindgen`
> flags until we have an alternative for that)
The bit that worries me most is bindgen, and in particular detecting the
version of libclang used. I mentioned to Nathan or Nick about needing a
buildtime test for the version of LIBCLANG being used.
I'm less worried about this for LLVM=1 builds, since while I think it is
possible to provide a LIBCLANG path to the build system, I suspect that
for LLVM=1 builds it's almost always going to match the LLVM toolchain
in use.
> > When it comes to asymmetrical extension support between the C and Rust
> > toolchains, I'm think we deal with that as we do for the C toolchains,
> > sort issues out as-and-when they arrive rather than punt this again.
>
> Sounds good, thanks a lot!
I'll do another rebase and resend after the merge window closes I
suppose :)
On Thu, Jan 18, 2024 at 4:49 PM Conor Dooley <[email protected]> wrote:
>
> The bit that worries me most is bindgen, and in particular detecting the
> version of libclang used. I mentioned to Nathan or Nick about needing a
> buildtime test for the version of LIBCLANG being used.
> I'm less worried about this for LLVM=1 builds, since while I think it is
> possible to provide a LIBCLANG path to the build system, I suspect that
> for LLVM=1 builds it's almost always going to match the LLVM toolchain
> in use.
`scripts/rust_is_available.sh` tests whether `libclang` is at least
the minimum LLVM supported version; and under `LLVM=1` builds, it also
tests whether the `bindgen` found one matches the C compiler. Do you
mean something like that?
For `bindgen` under GCC builds, we will eventually want a "proper" way
to use GCC instead (or possibly other approaches like querying the
information): https://github.com/rust-lang/rust-bindgen/issues/1949.
Recently, there has been a thread in our Zulip and a couple people are
experimenting: https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Bindgen.20--.20GCC.20backend.20port
> I'll do another rebase and resend after the merge window closes I
> suppose :)
Thanks!
Cheers,
Miguel
On Thu, Jan 18, 2024 at 05:09:40PM +0100, Miguel Ojeda wrote:
> On Thu, Jan 18, 2024 at 4:49 PM Conor Dooley <[email protected]> wrote:
> >
> > The bit that worries me most is bindgen, and in particular detecting the
> > version of libclang used. I mentioned to Nathan or Nick about needing a
> > buildtime test for the version of LIBCLANG being used.
> > I'm less worried about this for LLVM=1 builds, since while I think it is
> > possible to provide a LIBCLANG path to the build system, I suspect that
> > for LLVM=1 builds it's almost always going to match the LLVM toolchain
> > in use.
I chatted with the clang built linux folks about this yesterday, Nathan
agreed that dealing with incompatibility issues iff they crop up is a
reasonable way to go.
> `scripts/rust_is_available.sh` tests whether `libclang` is at least
> the minimum LLVM supported version; and under `LLVM=1` builds, it also
> tests whether the `bindgen` found one matches the C compiler. Do you
> mean something like that?
If by "the bindgen found one matches the C compiler" you mean that the
version of libclang used by bindgen matches the C compiler, then that
sounds great.
> For `bindgen` under GCC builds, we will eventually want a "proper" way
> to use GCC instead (or possibly other approaches like querying the
> information): https://github.com/rust-lang/rust-bindgen/issues/1949.
> Recently, there has been a thread in our Zulip and a couple people are
> experimenting: https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Bindgen.20--.20GCC.20backend.20port
That link for me goes to a message on 22/01, so later than the email you
sent.
> > I'll do another rebase and resend after the merge window closes I
> > suppose :)
That said, I gave things another spin today, in a different environment,
as a final check before sending and found an issue causing kernel
panics. RISC-V (and x86/arm64) supports kcfi (CFI_CLANG) but enabling
sanitisers seems to be a nightly only option for rustc. The kernel I
built today had CFI_CLANG enabled and that caused panics when the rust
samples were loaded.
The CFI_CLANG Kconfig entry has a cc-option test for whether the option
is supported, but from a quick check I don't see a comparable test to
use for rust. Even if a test was added, the current flag is an unstable
one, so I am not sure if testing for it is the right call in the first
place, given the stabilised flag would be entirely different?
The tracking issue seems to be complete:
https://github.com/rust-lang/rust/issues/89653
but the tracking issue for sanitisiers themselves is only 3/5:
https://github.com/rust-lang/rust/issues/39699
The simple thing would be to make them mutually exclusive options in
Kconfig.
What do you think?
Cheers,
Conor.
On Thu, Jan 25, 2024 at 1:31 PM Conor Dooley <[email protected]> wrote:
>
> I chatted with the clang built linux folks about this yesterday, Nathan
> agreed that dealing with incompatibility issues iff they crop up is a
> reasonable way to go.
>
> If by "the bindgen found one matches the C compiler" you mean that the
> version of libclang used by bindgen matches the C compiler, then that
> sounds great.
Yeah, exactly. So, unless I am misunderstanding, the incompatibilities
could only happen if someone ignores the warning. We could make it an
error too.
> > For `bindgen` under GCC builds, we will eventually want a "proper" way
> > to use GCC instead (or possibly other approaches like querying the
> > information): https://github.com/rust-lang/rust-bindgen/issues/1949.
>
> > Recently, there has been a thread in our Zulip and a couple people are
> > experimenting: https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Bindgen.20--.20GCC.20backend.20port
>
> That link for me goes to a message on 22/01, so later than the email you
> sent.
Zulip seems to scroll to the latest message in the topic -- you should
be able to scroll a bit up, but if that doesn't work, this link should
go to the first message:
https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Bindgen.20--.20GCC.20backend.20port/near/412609074
> That said, I gave things another spin today, in a different environment,
> as a final check before sending and found an issue causing kernel
> panics. RISC-V (and x86/arm64) supports kcfi (CFI_CLANG) but enabling
> sanitisers seems to be a nightly only option for rustc. The kernel I
> built today had CFI_CLANG enabled and that caused panics when the rust
> samples were loaded.
>
> The CFI_CLANG Kconfig entry has a cc-option test for whether the option
> is supported, but from a quick check I don't see a comparable test to
> use for rust. Even if a test was added, the current flag is an unstable
> one, so I am not sure if testing for it is the right call in the first
> place, given the stabilised flag would be entirely different?
Yeah, KCFI and other mitigations is WIP -- Cc'ing Ramon and Matthew
who may be able to tell us the latest status.
Testing for unstable flags is fine, i.e. we only support a single
compiler, so we can change the name when we do the upgrade.
Cheers,
Miguel
On Thu, Jan 25, 2024 at 01:50:05PM +0100, Miguel Ojeda wrote:
> On Thu, Jan 25, 2024 at 1:31 PM Conor Dooley <[email protected]> wrote:
> > > Recently, there has been a thread in our Zulip and a couple people are
> > > experimenting: https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Bindgen.20--.20GCC.20backend.20port
> >
> > That link for me goes to a message on 22/01, so later than the email you
> > sent.
>
> Zulip seems to scroll to the latest message in the topic -- you should
> be able to scroll a bit up, but if that doesn't work, this link should
> go to the first message:
> https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/Bindgen.20--.20GCC.20backend.20port/near/412609074
Ah, thanks for the direct link :)
>
> > That said, I gave things another spin today, in a different environment,
> > as a final check before sending and found an issue causing kernel
> > panics. RISC-V (and x86/arm64) supports kcfi (CFI_CLANG) but enabling
I mention x86 and arm64 here, because my grepping didn't see the flag
being set for x86 (in tree) or arm64 (in that series) if CFI_CLANG was
set or any mutual exclusion. Has noone tried CFI_CLANG + RUST there or
just not run into any issues?
> > sanitisers seems to be a nightly only option for rustc. The kernel I
> > built today had CFI_CLANG enabled and that caused panics when the rust
> > samples were loaded.
> >
> > The CFI_CLANG Kconfig entry has a cc-option test for whether the option
> > is supported, but from a quick check I don't see a comparable test to
> > use for rust. Even if a test was added, the current flag is an unstable
> > one, so I am not sure if testing for it is the right call in the first
> > place, given the stabilised flag would be entirely different?
>
> Yeah, KCFI and other mitigations is WIP -- Cc'ing Ramon and Matthew
> who may be able to tell us the latest status.
Also CC Sami I guess, since he is the one who added the CFI_CLANG bits
to the kernel, and can probably comment on the suitability of adding a
check etc.
> Testing for unstable flags is fine, i.e. we only support a single
> compiler, so we can change the name when we do the upgrade.
Actually, thinking about it for a moment - if only a single compiler
version is supported (the minimum, right?) then you could just add the
-Zsanitizer=kcfi flag whenever CFI_CLANG and RUST are both set.
I'm not sure if that is a better option though. It's a choice between
CFI_CLANG being disabled if the check is not updated when the toolchain
is bumped versus being enabled for C and not for RUST. I think I prefer
the former though, tracking down the cause of the latter I would rather
not wish on a user.
I vote for having the check, even if it can only ever be true at the
moment.
Cheers,
Conor.
On Thu, Jan 25, 2024 at 2:46 PM Conor Dooley <[email protected]> wrote:
>
> Ah, thanks for the direct link :)
My pleasure!
> Actually, thinking about it for a moment - if only a single compiler
> version is supported (the minimum, right?) then you could just add the
Yeah, the minimum listed in `scripts/min-tool-version.sh` and in
`Documentation/process/changes.rst`. It also happens to be the maximum
too, until we can relax that.
> -Zsanitizer=kcfi flag whenever CFI_CLANG and RUST are both set.
Since the flag goes to the Rust compiler, `RUST` would be always
enabled, so the flag would only need to be added when `CFI_CLANG=y`,
no? Or what do you mean?
> I'm not sure if that is a better option though. It's a choice between
> CFI_CLANG being disabled if the check is not updated when the toolchain
> is bumped versus being enabled for C and not for RUST. I think I prefer
> the former though, tracking down the cause of the latter I would rather
> not wish on a user.
>
> I vote for having the check, even if it can only ever be true at the
> moment.
Since we only support a single version, we don't need `rc-option`
tests until we start supporting several versions (which is why other
tests like that do not exist so far).
In my previous message I thought you meant using the flag to test for
arch/target support or similar. That would be fine, but we can also do
the usual `ARCH_SUPPORTS_CFI_RUST` here, I would assume.
Now, during the version bump to a stable flag, if we happen to forget
to update the flag name, it would be a build error, so it should be
easily spotted and fixed. And if we did use an `rc-option` for the
arch/target support idea, it would be the first case you mention, so
it is all good.
What we may want to add, though, to avoid the confusion you mention
meanwhile, is just a `depends on !CFI_CLANG` for `RUST`, like for the
other requirements we have there (which are things that should
eventually go away). Then they can remove that when the `-Z` flag is
deemed ready to be used. But perhaps let's see what Ramon et al. say.
In other words, the flag was added back in 1.68.0 to `rustc`, but it
was not ready, so we need to store the "ready" bit in our side
meanwhile, i.e. we can't know that just by testing the flag itself.
By the way, concerning the tracking issue, since you mentioned it: it
has a list of PRs, but not fixes, there is a "known issues" link
there. On top of that, we are "shifted in time" w.r.t. the latest
status in the compiler, since we use stable versions of the compiler.
Cheers,
Miguel
On Fri, Jan 26, 2024 at 10:00:02PM +0100, Miguel Ojeda wrote:
> On Thu, Jan 25, 2024 at 2:46 PM Conor Dooley <[email protected]> wrote:
> >
> > Ah, thanks for the direct link :)
>
> My pleasure!
>
> > Actually, thinking about it for a moment - if only a single compiler
> > version is supported (the minimum, right?) then you could just add the
>
> Yeah, the minimum listed in `scripts/min-tool-version.sh` and in
> `Documentation/process/changes.rst`. It also happens to be the maximum
> too, until we can relax that.
>
> > -Zsanitizer=kcfi flag whenever CFI_CLANG and RUST are both set.
>
> Since the flag goes to the Rust compiler, `RUST` would be always
> enabled, so the flag would only need to be added when `CFI_CLANG=y`,
> no?
Sure.
> Or what do you mean?
Oh I was probably just getting myself mixed up between what the Kconfig
and Makefile stuff would look like. dw :)
> > I'm not sure if that is a better option though. It's a choice between
> > CFI_CLANG being disabled if the check is not updated when the toolchain
> > is bumped versus being enabled for C and not for RUST. I think I prefer
> > the former though, tracking down the cause of the latter I would rather
> > not wish on a user.
> >
> > I vote for having the check, even if it can only ever be true at the
> > moment.
>
> Since we only support a single version, we don't need `rc-option`
> tests until we start supporting several versions (which is why other
> tests like that do not exist so far).
>
> In my previous message I thought you meant using the flag to test for
> arch/target support or similar. That would be fine, but we can also do
> the usual `ARCH_SUPPORTS_CFI_RUST` here, I would assume.
Is that even needed? We already have ARCH_SUPPORTS_CFI_CLANG and AFAIU
rust supports it if clang does, so a second option is superfluous?
> Now, during the version bump to a stable flag, if we happen to forget
> to update the flag name, it would be a build error, so it should be
> easily spotted and fixed.
I'm reading back what I wrote, and I must have been trying to get out
the door or something because none of it really makes that much sense.
Of course an unknown option should be detectable at build time and not
be a silent breakage. Maybe I should have written the patch for this
before sending the mail rather than writing the mail based on what was
in my head.
> What we may want to add, though, to avoid the confusion you mention
> meanwhile, is just a `depends on !CFI_CLANG` for `RUST`, like for the
> other requirements we have there (which are things that should
> eventually go away). Then they can remove that when the `-Z` flag is
> deemed ready to be used. But perhaps let's see what Ramon et al. say.
I don't really mind either way. Whatever of the two that you guys want
that prevents broken kernels works for me!
> By the way, concerning the tracking issue, since you mentioned it: it
> has a list of PRs, but not fixes, there is a "known issues" link
> there. On top of that, we are "shifted in time" w.r.t. the latest
> status in the compiler, since we use stable versions of the compiler.
Yah, I was linking it to point out that the stuff is unlikely to be
usable any time soon, since it is not complete in the most recent
toolchain.
Cheers,
Conor.
On Fri, Jan 26, 2024 at 11:01 PM Conor Dooley <[email protected]> wrote:
>
> Is that even needed? We already have ARCH_SUPPORTS_CFI_CLANG and AFAIU
> rust supports it if clang does, so a second option is superfluous?
From a quick look, I don't see it enabled in any RISC-V built-in
target in `rustc` yet.
It may also still be the case that KCFI needs some tweaks for, say,
RISC-V, before the flag actually works, i.e. we couldn't just test the
flag in that case -- Ramon: how likely is it that RISC-V would work if
KCFI works for aarch64 and x86_64?
> I'm reading back what I wrote, and I must have been trying to get out
> the door or something because none of it really makes that much sense.
> Of course an unknown option should be detectable at build time and not
> be a silent breakage. Maybe I should have written the patch for this
> before sending the mail rather than writing the mail based on what was
> in my head.
No worries, it happens. I probably added to the confusion when I
replied with the "testing the unstable flag".
Cheers,
Miguel
On Sat, Jan 27, 2024 at 02:46:38PM +0100, Miguel Ojeda wrote:
> On Fri, Jan 26, 2024 at 11:01 PM Conor Dooley <[email protected]> wrote:
> >
> > Is that even needed? We already have ARCH_SUPPORTS_CFI_CLANG and AFAIU
> > rust supports it if clang does, so a second option is superfluous?
>
> From a quick look, I don't see it enabled in any RISC-V built-in
> target in `rustc` yet.
>
> It may also still be the case that KCFI needs some tweaks for, say,
> RISC-V, before the flag actually works, i.e. we couldn't just test the
> flag in that case -- Ramon: how likely is it that RISC-V would work if
> KCFI works for aarch64 and x86_64?
Well, there's been no reply here. I'll do sa you suggested and add a
depends on !CFI_CLANG to RUST.
Cheers,
Conor.
On Fri, Feb 9, 2024 at 9:18 AM Conor Dooley <[email protected]> wrote:
>
> On Sat, Jan 27, 2024 at 02:46:38PM +0100, Miguel Ojeda wrote:
> > On Fri, Jan 26, 2024 at 11:01 PM Conor Dooley <[email protected]> wrote:
> > >
> > > Is that even needed? We already have ARCH_SUPPORTS_CFI_CLANG and AFAIU
> > > rust supports it if clang does, so a second option is superfluous?
> >
> > From a quick look, I don't see it enabled in any RISC-V built-in
> > target in `rustc` yet.
> >
> > It may also still be the case that KCFI needs some tweaks for, say,
> > RISC-V, before the flag actually works, i.e. we couldn't just test the
> > flag in that case -- Ramon: how likely is it that RISC-V would work if
> > KCFI works for aarch64 and x86_64?
>
> Well, there's been no reply here. I'll do sa you suggested and add a
> depends on !CFI_CLANG to RUST.
>
> Cheers,
> Conor.
>
I asked on Zulip and it sounds like Ramon may be out [1]. It
_probably_ works, but going with a dependency to not be blocked on
KCFI is probably reasonable for now.
- Trevor
[1]: https://rust-lang.zulipchat.com/#narrow/stream/343119-project-exploit-mitigations/topic/KCFI.20on.20RISC-V.20questions
Sorry for the late reply. Sami might be the best person to answer
this, but KCFI (not CFI) tests are lowered by passes that are
architecture specific (see https://reviews.llvm.org/D119296), so we'd
need to add support for RISC-V. There is no additional work required
in the Rust compiler besides enabling it for the new target.
On Sat, Feb 10, 2024 at 12:13 AM Trevor Gross <[email protected]> wrote:
>
> On Fri, Feb 9, 2024 at 9:18 AM Conor Dooley <[email protected]> wrote:
> >
> > On Sat, Jan 27, 2024 at 02:46:38PM +0100, Miguel Ojeda wrote:
> > > On Fri, Jan 26, 2024 at 11:01 PM Conor Dooley <[email protected]> wrote:
> > > >
> > > > Is that even needed? We already have ARCH_SUPPORTS_CFI_CLANG and AFAIU
> > > > rust supports it if clang does, so a second option is superfluous?
> > >
> > > From a quick look, I don't see it enabled in any RISC-V built-in
> > > target in `rustc` yet.
> > >
> > > It may also still be the case that KCFI needs some tweaks for, say,
> > > RISC-V, before the flag actually works, i.e. we couldn't just test the
> > > flag in that case -- Ramon: how likely is it that RISC-V would work if
> > > KCFI works for aarch64 and x86_64?
> >
> > Well, there's been no reply here. I'll do sa you suggested and add a
> > depends on !CFI_CLANG to RUST.
> >
> > Cheers,
> > Conor.
> >
>
> I asked on Zulip and it sounds like Ramon may be out [1]. It
> _probably_ works, but going with a dependency to not be blocked on
> KCFI is probably reasonable for now.
>
> - Trevor
>
> [1]: https://rust-lang.zulipchat.com/#narrow/stream/343119-project-exploit-mitigations/topic/KCFI.20on.20RISC-V.20questions
On Mon, Feb 12, 2024 at 8:02 PM Ramon de C Valle <[email protected]> wrote:
>
> Sorry for the late reply. Sami might be the best person to answer this, but KCFI (not CFI) tests are lowered by passes that are architecture specific (see https://reviews.llvm.org/D119296), so we'd need to add support for RISC-V. There is no additional work required in the Rust compiler besides enabling it for the new target.
Thanks a lot Ramon!
Then for RISC-V let's go for the `depends on` for the moment, and we
can remove when the support lands for RISC-V (ideally when someone has
managed to boot it at least under some configuration).
Cheers,
Miguel
On Mon, Feb 12, 2024 at 08:11:21PM +0100, Miguel Ojeda wrote:
> On Mon, Feb 12, 2024 at 8:02 PM Ramon de C Valle <[email protected]> wrote:
> >
> > Sorry for the late reply. Sami might be the best person to answer this, but KCFI (not CFI) tests are lowered by passes that are architecture specific (see https://reviews.llvm.org/D119296), so we'd need to add support for RISC-V. There is no additional work required in the Rust compiler besides enabling it for the new target.
>
> Thanks a lot Ramon!
>
> Then for RISC-V let's go for the `depends on` for the moment, and we
> can remove when the support lands for RISC-V (ideally when someone has
> managed to boot it at least under some configuration).
If all you want is a boot under some configuration, that's not
difficult. After all, I found the original issue by booting a kernel
with CFI_CLANG enabled on the C side...
> There is no additional work required in the Rust compiler besides enabling it for the new target.
This is not super clear though, it says "in the Rust compiler", not "in
the kernel's buildsystem".
On Mon, Feb 12, 2024 at 11:04 AM Ramon de C Valle <[email protected]> wrote:
>
> Sorry for the late reply. Sami might be the best person to answer
> this, but KCFI (not CFI) tests are lowered by passes that are
> architecture specific (see https://reviews.llvm.org/D119296), so we'd
> need to add support for RISC-V. There is no additional work required
> in the Rust compiler besides enabling it for the new target.
LLVM 17 added KCFI support for RISC-V. Sounds like it's just a
question of whether rustc uses a new enough version of LLVM then?
Sami
On Mon, Feb 12, 2024 at 08:17:31PM +0000, Conor Dooley wrote:
> On Mon, Feb 12, 2024 at 08:11:21PM +0100, Miguel Ojeda wrote:
> > On Mon, Feb 12, 2024 at 8:02 PM Ramon de C Valle <[email protected]> wrote:
> > >
> > > Sorry for the late reply. Sami might be the best person to answer this, but KCFI (not CFI) tests are lowered by passes that are architecture specific (see https://reviews.llvm.org/D119296), so we'd need to add support for RISC-V. There is no additional work required in the Rust compiler besides enabling it for the new target.
> >
> > Thanks a lot Ramon!
> >
> > Then for RISC-V let's go for the `depends on` for the moment, and we
> > can remove when the support lands for RISC-V (ideally when someone has
> > managed to boot it at least under some configuration).
>
> If all you want is a boot under some configuration, that's not
> difficult. After all, I found the original issue by booting a kernel
> with CFI_CLANG enabled on the C side...
Also, regardless of depends on on RISC-V, things will still be broken
on arm64 and x86_64, since KCFI is not enabled in rustc there either?
> > There is no additional work required in the Rust compiler besides enabling it for the new target.
>
> This is not super clear though, it says "in the Rust compiler", not "in
> the kernel's buildsystem".
I realise I was not clear either. What I meant was that this talks about
rustc and not kbuild, so what is meant by "the new target" is not clear.
Do arm64 and x86_64 have functional support, so adding RISC-V in rustc
is needed, or did you mean for the new target in the kernel?
> I realise I was not clear either. What I meant was that this talks about
> rustc and not kbuild, so what is meant by "the new target" is not clear.
> Do arm64 and x86_64 have functional support, so adding RISC-V in rustc
> is needed, or did you mean for the new target in the kernel?
Sorry, I was referring to the targets at:
https://github.com/rust-lang/rust/tree/master/compiler/rustc_target/src/spec/targets
On Tue, Feb 13, 2024 at 3:09 PM Ramon de C Valle <[email protected]> wrote:
>
> On Mon, Feb 12, 2024 at 12:36 PM Sami Tolvanen <[email protected]> wrote:
> >
> > On Mon, Feb 12, 2024 at 11:04 AM Ramon de C Valle <[email protected]> wrote:
> > >
> > > Sorry for the late reply. Sami might be the best person to answer
> > > this, but KCFI (not CFI) tests are lowered by passes that are
> > > architecture specific (see https://reviews.llvm.org/D119296), so we'd
> > > need to add support for RISC-V. There is no additional work required
> > > in the Rust compiler besides enabling it for the new target.
> >
> > LLVM 17 added KCFI support for RISC-V. Sounds like it's just a
> > question of whether rustc uses a new enough version of LLVM then?
>
> rustc is using LLVM 17 since
> https://github.com/rust-lang/rust/pull/114048, so I guess I can enable
> it for RISC-V targets in
> https://github.com/rust-lang/rust/tree/master/compiler/rustc_target/src/spec/targets,
> and the Linux kernel will get it on the next compiler update?
18 as of a few hours ago :) https://github.com/rust-lang/rust/pull/120055
Just for an idea of timing, anything merged this month will be in 1.78
on 2024-05-02.