2024-02-23 13:39:12

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 0/3] RISC-V: enable rust

From: Conor Dooley <[email protected]>

Now with a patch to disable RUST if CFI_CLANG is enabled.
I've also intentionally not turned on the gcc support, as discussed on
v1.

As this was 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 v2:
- Rebase, since a good bit of time has passed!
- Add the extra patch, disabling when CFI_CLANG is enabled.

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]

Conor Dooley (1):
rust: make mutually exclusive with CFI_CLANG

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 | 1 +
arch/riscv/Kconfig | 1 +
arch/riscv/Makefile | 2 ++
init/Kconfig | 1 +
scripts/generate_rust_target.rs | 16 ++++++++++++++++
5 files changed, 21 insertions(+)

--
2.43.0



2024-02-23 13:39:29

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 1/3] rust: make mutually exclusive with CFI_CLANG

From: Conor Dooley <[email protected]>

On RISC-V, and presumably x86/arm64, if CFI_CLANG is enabled loading a
rust module will trigger a kernel panic. Support for sanitisers,
including kcfi (CFI_CLANG), is in the works, but for now they're
nightly-only options in rustc. Make RUST depend on !CFI_CLANG to prevent
configuring a kernel without symmetrical support for kfi.

Fixes: 2f7ab1267dc9 ("Kbuild: add Rust support")
cc: [email protected]
Signed-off-by: Conor Dooley <[email protected]>
---
This probably needs to go to stable. The correct fixes tag for that I am
not sure of however, but since CFI_CLANG predates RUST, I blamed the
commit adding rust support.
---
init/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/init/Kconfig b/init/Kconfig
index 8d4e836e1b6b..6cf05824859e 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1895,6 +1895,7 @@ config RUST
bool "Rust support"
depends on HAVE_RUST
depends on RUST_IS_AVAILABLE
+ depends on !CFI_CLANG
depends on !MODVERSIONS
depends on !GCC_PLUGINS
depends on !RANDSTRUCT
--
2.43.0


2024-02-23 13:39:45

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

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]>
---
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 416c6b89e806..942ddca57ee4 100644
--- a/scripts/generate_rust_target.rs
+++ b/scripts/generate_rust_target.rs
@@ -171,6 +171,22 @@ fn main() {
ts.push("llvm-target", "loongarch64-linux-gnusf");
ts.push("llvm-abiname", "lp64s");
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.43.0


2024-02-23 13:40:02

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 3/3] RISC-V: enable building 64-bit kernels with rust support

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]>
---
Documentation/rust/arch-support.rst | 1 +
arch/riscv/Kconfig | 1 +
arch/riscv/Makefile | 2 ++
3 files changed, 4 insertions(+)

diff --git a/Documentation/rust/arch-support.rst b/Documentation/rust/arch-support.rst
index 73203ba1e901..9e18a81fc2ef 100644
--- a/Documentation/rust/arch-support.rst
+++ b/Documentation/rust/arch-support.rst
@@ -16,6 +16,7 @@ support corresponds to ``S`` values in the ``MAINTAINERS`` file.
Architecture Level of support Constraints
============= ================ ==============================================
``loongarch`` Maintained -
+``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 5c59e00405e3..3eaae08e1d5c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -142,6 +142,7 @@ config RISCV
select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_RETHOOK if !XIP_KERNEL
select HAVE_RSEQ
+ select HAVE_RUST if 64BIT
select HAVE_STACKPROTECTOR
select HAVE_SYSCALL_TRACEPOINTS
select HOTPLUG_CORE_SYNC_DEAD if HOTPLUG_CPU
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index ebbe02628a27..22fdb1e83744 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -34,6 +34,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.43.0


2024-02-23 14:43:11

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Fri, Feb 23, 2024 at 2:38 PM Conor Dooley <[email protected]> wrote:
>
> 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.

Thanks for this Connor!

arm64 is sending these for 6.9:

https://git.kernel.org/arm64/c/f82811e22b48
https://git.kernel.org/arm64/c/724a75ac9542

So it would be nice to see if it may be already possible to enable it
via a builtin target + flags instead of the custom target, e.g. arm64
does:

KBUILD_RUSTFLAGS += --target=aarch64-unknown-none -Ctarget-feature="-neon"

and so on.

If it does not work, it would be good to know what would be needed for
RISC-V and put it into the unstable features / wanted features list
for Rust.

Either way, it is not a blocker (although you will need a rebase after
arm64 lands to use the `target.json` in the right places).

Cheers,
Miguel

2024-02-23 14:46:26

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] rust: make mutually exclusive with CFI_CLANG

On Fri, Feb 23, 2024 at 2:38 PM Conor Dooley <[email protected]> wrote:
>
> configuring a kernel without symmetrical support for kfi.

Nit: typo.

> This probably needs to go to stable. The correct fixes tag for that I am
> not sure of however, but since CFI_CLANG predates RUST, I blamed the
> commit adding rust support.

Cc'ing Matthew et al. in case this is a problem for them, but I guess
we can relax it later as needed.

Cheers,
Miguel

2024-02-23 14:50:44

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] RISC-V: enable building 64-bit kernels with rust support

On Fri, Feb 23, 2024 at 2:38 PM Conor Dooley <[email protected]> wrote:
>
> 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.

Nit: this can be removed now.

Cheers,
Miguel

2024-02-27 10:18:05

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Fri, Feb 23, 2024 at 03:39:47PM +0100, Miguel Ojeda wrote:

> Conor Dooley <[email protected]> wrote:

> Thanks for this Connor!




> arm64 is sending these for 6.9:
>
> https://git.kernel.org/arm64/c/f82811e22b48
> https://git.kernel.org/arm64/c/724a75ac9542
>
> So it would be nice to see if it may be already possible to enable it
> via a builtin target + flags instead of the custom target, e.g. arm64
> does:
>
> KBUILD_RUSTFLAGS += --target=aarch64-unknown-none -Ctarget-feature="-neon"
>
> and so on.
>
> If it does not work, it would be good to know what would be needed for
> RISC-V and put it into the unstable features / wanted features list
> for Rust.

Sure, I'll take a look.

> Either way, it is not a blocker (although you will need a rebase after
> arm64 lands to use the `target.json` in the right places).

Nah, I think that is silly. Either this goes in as-is, and there's
fixup done by Linus, or the thing should be converted to match arm64,
assuming that that is possible.

Cheers,
Conor.


Attachments:
(No filename) (1.04 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-27 10:49:34

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Tue, Feb 27, 2024 at 11:17 AM Conor Dooley
<[email protected]> wrote:
>
> Sure, I'll take a look.

Thanks!

> Nah, I think that is silly. Either this goes in as-is, and there's
> fixup done by Linus, or the thing should be converted to match arm64,
> assuming that that is possible.

Ah, so you are going for 6.9 too? I can give the series a try on my
side in that case. When do you plan to apply them?

Cheers,
Miguel

2024-02-27 10:56:23

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] rust: make mutually exclusive with CFI_CLANG

On Fri, Feb 23, 2024 at 03:45:10PM +0100, Miguel Ojeda wrote:
> On Fri, Feb 23, 2024 at 2:38 PM Conor Dooley <[email protected]> wrote:
> >
> > configuring a kernel without symmetrical support for kfi.
>
> Nit: typo.
>
> > This probably needs to go to stable. The correct fixes tag for that I am
> > not sure of however, but since CFI_CLANG predates RUST, I blamed the
> > commit adding rust support.
>
> Cc'ing Matthew et al. in case this is a problem for them, but I guess
> we can relax it later as needed.

I suspect that nobody has actually sat down and tried it.

I did try to test it but I ran into too many toolchain issues - my
older copies of LLVM (pre 17) are not multiarch as I built them by hand
with PGO for x86 and RISC-V. My LLVM 17 is from kernel.org and has no
libclang. And then the copy of LLVM 18 on kernel.org apparently does not
support kcfi at all. I gave up there, but I don't see how this would not
be a problem on other arches, given rustc never gets told to enable
kcfi.

Cheers,
Conor.


Attachments:
(No filename) (1.02 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-27 11:06:09

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Tue, Feb 27, 2024 at 11:46:42AM +0100, Miguel Ojeda wrote:
> On Tue, Feb 27, 2024 at 11:17 AM Conor Dooley
> <[email protected]> wrote:
> >
> > Sure, I'll take a look.
>
> Thanks!
>
> > Nah, I think that is silly. Either this goes in as-is, and there's
> > fixup done by Linus, or the thing should be converted to match arm64,
> > assuming that that is possible.
>
> Ah, so you are going for 6.9 too? I can give the series a try on my
> side in that case. When do you plan to apply them?

If they're to be applied, it would be Palmer, not I. And if history is
any guide, it could be into the merge window. My point though was more
that either this was acceptable for v6.9 or would be v6.10 material
with the same mechanism as arm64. Rebasing after v6.9-rc1 but not
adapting to that way of doing things is what seemed silly to me, since
if a resend is required then the other improvements should be carried
out at the same time.

Cheers,
Conor.


Attachments:
(No filename) (986.00 B)
signature.asc (235.00 B)
Download all attachments

2024-02-27 12:19:44

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Tue, Feb 27, 2024 at 12:05 PM Conor Dooley
<[email protected]> wrote:
>
> My point though was more
> that either this was acceptable for v6.9 or would be v6.10 material
> with the same mechanism as arm64. Rebasing after v6.9-rc1 but not
> adapting to that way of doing things is what seemed silly to me, since
> if a resend is required then the other improvements should be carried
> out at the same time.

If avoiding the `target.json` is possible, definitely.

I didn't want to assume it is, though -- e.g. the native integer
widths you have is 64 but the built-in targets use 32:64 (perhaps
there is a way to tweak it with an LLVM param via `-Cllvm-args`, but I
don't see any obvious way from a quick look; `opt` does have it,
though).

(That is why we supported `target.json`, since it gives the most
freedom in the beginning.)

Cheers,
Miguel

2024-02-27 12:34:41

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] rust: make mutually exclusive with CFI_CLANG

On Tue, Feb 27, 2024 at 11:54 AM Conor Dooley
<[email protected]> wrote:
>
> I did try to test it but I ran into too many toolchain issues - my
> older copies of LLVM (pre 17) are not multiarch as I built them by hand
> with PGO for x86 and RISC-V. My LLVM 17 is from kernel.org and has no
> libclang. And then the copy of LLVM 18 on kernel.org apparently does not
> support kcfi at all. I gave up there, but I don't see how this would not

I asked Nathan to add libclang a few days ago, and he very quickly did
it for LLVM 18 -- though I don't know the plan for the others. I just
pinged in that thread.

Cheers,
Miguel

2024-02-27 12:37:39

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Tue, Feb 27, 2024 at 01:12:51PM +0100, Miguel Ojeda wrote:
> On Tue, Feb 27, 2024 at 12:05 PM Conor Dooley
> <[email protected]> wrote:
> >
> > My point though was more
> > that either this was acceptable for v6.9 or would be v6.10 material
> > with the same mechanism as arm64. Rebasing after v6.9-rc1 but not
> > adapting to that way of doing things is what seemed silly to me, since
> > if a resend is required then the other improvements should be carried
> > out at the same time.
>
> If avoiding the `target.json` is possible, definitely.
>
> I didn't want to assume it is, though -- e.g. the native integer
> widths you have is 64 but the built-in targets use 32:64 (perhaps
> there is a way to tweak it with an LLVM param via `-Cllvm-args`, but I
> don't see any obvious way from a quick look; `opt` does have it,
> though).

Looking closer at those targets, all of them enable compressed
instructors, but we support hardware that does not support them.
I think that means we are stuck with the custom targets.

I could badger Palmer to pick this up tomorrow, provided you're okay
with the gist of this series.

Cheers,
Conor.


Attachments:
(No filename) (1.15 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-27 13:03:38

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] rust: make mutually exclusive with CFI_CLANG

On Tue, Feb 27, 2024 at 01:34:14PM +0100, Miguel Ojeda wrote:
> On Tue, Feb 27, 2024 at 11:54 AM Conor Dooley
> <[email protected]> wrote:
> >
> > I did try to test it but I ran into too many toolchain issues - my
> > older copies of LLVM (pre 17) are not multiarch as I built them by hand
> > with PGO for x86 and RISC-V. My LLVM 17 is from kernel.org and has no
> > libclang. And then the copy of LLVM 18 on kernel.org apparently does not
> > support kcfi at all. I gave up there, but I don't see how this would not
>
> I asked Nathan to add libclang a few days ago, and he very quickly did
> it for LLVM 18 -- though I don't know the plan for the others. I just
> pinged in that thread.

I had actually said it to him on IRC already (although he is CCed here)
but I just noticed that this was my fault - I symlinked incorrectly
after downloading the toolchain. kcfi is detected fine with llvm18.
I'll give testing another try.


Attachments:
(No filename) (960.00 B)
signature.asc (235.00 B)
Download all attachments

2024-02-27 13:33:18

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] rust: make mutually exclusive with CFI_CLANG

On Tue, Feb 27, 2024 at 01:02:17PM +0000, Conor Dooley wrote:

> I'll give testing another try.

Yah, it is (as expected) broken on arm64 too:

CFI failure at do_one_initcall+0xec/0x26c (target: __rust_minimal_init+0x0/0x64; expected type: 0x36b1c5a6)
Internal error: Oops - CFI: 00000000f2008233 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc3-00002-g724a75ac9542 #10
Hardware name: linux,dummy-virt (DT)
pstate: a0000005 (NzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : do_one_initcall+0xec/0x26c
lr : do_initcall_level+0x8c/0xb0
sp : ffff80008000bab0
x29: ffff80008000bda0 x28: 0000000000000000 x27: 0000000000000000
x26: 0000000000000000 x25: 0000000000000000 x24: 0000000000000000
x23: 0000000000000000 x22: 0000000000000000 x21: ffff18d941cb0000
x20: ffffb8f6d6d9f000 x19: ffffb8f6d63e8b18 x18: 0000000000000002
x17: 0000000036b1c5a6 x16: 00000000d65f03c0 x15: 0000000000000000
x14: ffff18d9420280b1 x13: 0000000065dde380 x12: 0000000000000017
x11: 0000000000000000 x10: 0000000000000000 x9 : d4db8c0058e7e300
x8 : 0000000000000000 x7 : 0000000001f4c18b x6 : 0000000001f4c18b
x5 : ffffb8f6d6d300a0 x4 : ffff80008000ba78 x3 : ffff18d942019860
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffb8f6d63e8b18
Call trace:
do_one_initcall+0xec/0x26c
do_initcall_level+0x8c/0xb0
do_initcalls+0x54/0x94
do_basic_setup+0x68/0x78
kernel_init_freeable+0x100/0x16c
kernel_init+0x20/0x1a0
ret_from_fork+0x10/0x20
Code: 7298b4d1 72a6d631 6b11021f 54000040 (d4304660)
---[ end trace 0000000000000000 ]---
note: swapper/0[1] exited with irqs disabled
note: swapper/0[1] exited with preempt_count 1
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
Kernel Offset: 0x38f655800000 from 0xffff800080000000
PHYS_OFFSET: 0xffffe72700000000
CPU features: 0x0,88000203,3c020000,0100421b
Memory Limit: none
---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---

Cheers,
Conor.


Attachments:
(No filename) (2.00 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-27 14:49:38

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Tue, Feb 27, 2024 at 1:37 PM Conor Dooley <[email protected]> wrote:
>
> Looking closer at those targets, all of them enable compressed
> instructors, but we support hardware that does not support them.
> I think that means we are stuck with the custom targets.

Did you try `-Ctarget-feature=-c`? i.e. as far as I know, you can
disable target features even if they are enabled from the built-in.

It seems to work from a quick try (in userspace), e.g.

0000000000000000 <f>:
0: 9b 05 05 00 sext.w a1, a0
4: 13 05 a0 02 li a0, 0x2a
8: 63 84 05 00 beqz a1, 0x10 <f+0x10>
c: 13 05 b0 07 li a0, 0x7b
10: 67 80 00 00 ret

vs.

0000000000000000 <f>:
0: 9b 05 05 00 sext.w a1, a0
4: 13 05 a0 02 li a0, 0x2a
8: 99 c1 beqz a1, 0xe <f+0xe>
a: 13 05 b0 07 li a0, 0x7b
e: 82 80 ret

Cheers,
Miguel

2024-02-27 17:48:30

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Tue, Feb 27, 2024 at 03:47:29PM +0100, Miguel Ojeda wrote:

> Did you try `-Ctarget-feature=-c`? i.e. as far as I know, you can
> disable target features even if they are enabled from the built-in.

No, I didn't actually try anything. Between trying to test the kcfi
stuff on arm64 and the other work I was doing today I did not have a
chance to actually play with that yet. It comes down to you though I
suppose - would you rather have generate_rust_target enable the
compressed instructions depending on the config option or have the
Makefile disable it if compressed instructions are not enabled and use
a builtin target?


Attachments:
(No filename) (640.00 B)
signature.asc (235.00 B)
Download all attachments

2024-02-27 18:13:07

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Tue, Feb 27, 2024 at 6:48 PM Conor Dooley <[email protected]> wrote:
>
> No, I didn't actually try anything. Between trying to test the kcfi
> stuff on arm64 and the other work I was doing today I did not have a
> chance to actually play with that yet.

Apologies, I didn't mean it that way -- I was just wondering if it
didn't work. No rush on my side (in fact, I thought this was for 6.10
anyway).

> It comes down to you though I
> suppose - would you rather have generate_rust_target enable the
> compressed instructions depending on the config option or have the
> Makefile disable it if compressed instructions are not enabled and use
> a builtin target?

So the custom target support is there for flexibility purposes: we
were told is that since the target spec is too tied to LLVM, it is
unlikely to get stabilized (at least as it is), and thus they
preferred that we ask for whatever flags in `rustc` would be needed to
tweak things in an existing builtin target (or add new built-in
targets if needed).

Thus, when a new architecture is added, the question is whether one
can already use the flags approach or not.

For instance, to disable the compressed instructions, from what I can
tell, the flag I mentioned seems to work, so that is fine. However,
for something like tweaking the data layout for `n64` instead of
`n32:64`, I am not aware of a way to do so via a flag (but I see newer
LLVM uses `n32:64`, so that may be the better one anyway:
https://godbolt.org/z/Eh4cfdeMr).

So it all depends on whether you are happy with what the flags
approach already give you.

I hope that clarifies a bit!

Cheers,
Miguel

2024-02-27 19:10:46

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] scripts: generate_rust_target: enable building on RISC-V

On Tue, Feb 27, 2024 at 07:11:17PM +0100, Miguel Ojeda wrote:
> On Tue, Feb 27, 2024 at 6:48 PM Conor Dooley <[email protected]> wrote:

> For instance, to disable the compressed instructions, from what I can
> tell, the flag I mentioned seems to work, so that is fine.

To me, using flags is a good fit given that's what we do elsewhere,
and there won't be a mix of stuff that is done conditionally in a
script and conditionally in a Makefile.

> However,
> for something like tweaking the data layout for `n64` instead of
> `n32:64`, I am not aware of a way to do so via a flag (but I see newer
> LLVM uses `n32:64`, so that may be the better one anyway:
> https://godbolt.org/z/Eh4cfdeMr).

Yeah, I had looked at the blame for the targets earlier today and
noticed that it had been changed. Sadly rustc's commit is lacking
any justification whatsoever for the change, so I was not going to
really comment on that until I had looked.

> So it all depends on whether you are happy with what the flags
> approach already give you.
>
> I hope that clarifies a bit!

Ye, thanks. I'll give it a go when I have a bit of time this week.


Attachments:
(No filename) (1.13 kB)
signature.asc (235.00 B)
Download all attachments