Re: [PATCH v1 0/2] RISC-V: enable rust

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3321 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]