mbox series

[RFC,0/2] RISC-V: enable rust

Message ID 20230224133609.2877396-1-conor.dooley@microchip.com (mailing list archive)
Headers show
Series RISC-V: enable rust | expand

Message

Conor Dooley Feb. 24, 2023, 1:36 p.m. UTC
This is a somewhat blind (and maybe foolish) attempt at enabling Rust
for RISC-V. I've tested this on Icicle, and the modules seem to work.
I'd like to play around with Rust on RISC-V, but I'm not interested in
using downstream kernels, so figured I should try and see what's
missing...
I've tagged this as RFC in case I've missed some "WAaaaa you can't do
this" somewhere :)

Thanks,
Conor.

CC: Miguel Ojeda <ojeda@kernel.org>
CC: Alex Gaynor <alex.gaynor@gmail.com>
CC: Wedson Almeida Filho <wedsonaf@gmail.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Gary Guo <gary@garyguo.net>
CC: Björn Roy Baron <bjorn3_gh@protonmail.com>
CC: Jonathan Corbet <corbet@lwn.net>
CC: Paul Walmsley <paul.walmsley@sifive.com>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Nathan Chancellor <nathan@kernel.org>
CC: Nick Desaulniers <ndesaulniers@google.com>
CC: Tom Rix <trix@redhat.com>
CC: rust-for-linux@vger.kernel.org
CC: linux-doc@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-riscv@lists.infradead.org
CC: llvm@lists.linux.dev

Miguel Ojeda (2):
  scripts: generate_rust_target: enable building on RISC-V
  RISC-V: enable building the 64-bit kernels with rust support

 Documentation/rust/arch-support.rst |  2 ++
 arch/riscv/Kconfig                  |  1 +
 arch/riscv/Makefile                 |  3 ++-
 scripts/generate_rust_target.rs     | 19 +++++++++++++++++++
 4 files changed, 24 insertions(+), 1 deletion(-)

Comments

Miguel Ojeda Feb. 24, 2023, 8:42 p.m. UTC | #1
Hi Conor,

On Fri, Feb 24, 2023 at 2:37 PM Conor Dooley <conor.dooley@microchip.com> wrote:
>
> This is a somewhat blind (and maybe foolish) attempt at enabling Rust
> for RISC-V. I've tested this on Icicle, and the modules seem to work.
> I'd like to play around with Rust on RISC-V, but I'm not interested in
> using downstream kernels, so figured I should try and see what's
> missing...
> I've tagged this as RFC in case I've missed some "WAaaaa you can't do
> this" somewhere :)

Thanks for sending this and taking the lead on RISC-V -- I appreciate
you put me as the author, but in this case, it was actually Gary that
started the RISC-V port [1], and then I sent three PRs on top later
on.

When submitting something on behalf of somebody else, I suggest being
very careful and ideally contacting the authors beforehand. In
particular, if there has been any modification (including to the
commit message), then a note should be added explaining so.

Therefore, please double-check everybody that contributed to the lines
you are sending (e.g. via `git-blame`) and add them as
`Co-developed-by`. Also, please add a note of what you changed/wrote
e.g. square brackets, and `Link` tags to the original PRs/discussions.

See, for instance, what Jamie did at [2] for arm64 or Lina at [3].

[1] https://github.com/Rust-for-Linux/linux/pull/225
[2] https://lore.kernel.org/rust-for-linux/20230125163739.3798252-2-Jamie.Cunliffe@arm.com/
[3] https://lore.kernel.org/rust-for-linux/20230224-rust-vec-v1-4-733b5b5a57c5@asahilina.net/

Cheers,
Miguel
Conor Dooley Feb. 24, 2023, 9 p.m. UTC | #2
On Fri, Feb 24, 2023 at 09:42:08PM +0100, Miguel Ojeda wrote:
> Hi Conor,
> 
> On Fri, Feb 24, 2023 at 2:37 PM Conor Dooley <conor.dooley@microchip.com> wrote:
> >
> > This is a somewhat blind (and maybe foolish) attempt at enabling Rust
> > for RISC-V. I've tested this on Icicle, and the modules seem to work.
> > I'd like to play around with Rust on RISC-V, but I'm not interested in
> > using downstream kernels, so figured I should try and see what's
> > missing...
> > I've tagged this as RFC in case I've missed some "WAaaaa you can't do
> > this" somewhere :)
> 
> Thanks for sending this and taking the lead on RISC-V

Meh, far from it. I'm just trying to get the ball rolling if it really
is as trivial as this seems.

> I appreciate
> you put me as the author, but in this case, it was actually Gary that
> started the RISC-V port [1], and then I sent three PRs on top later
> on.

The stuff that I have lifted here all had you as the sole author in the
"rust" branch downstream, which is why I gave you authorship. Namely:
afba78eacb9b ("rust: generate target specification files on the fly")
732b3c386328 ("rust: target: remove `cpu`")

I don't see anything from [1] in these commits, so I don't think that I
made a mistake here.

> When submitting something on behalf of somebody else, I suggest being
> very careful and ideally contacting the authors beforehand. In
> particular, if there has been any modification (including to the
> commit message), then a note should be added explaining so.

It's RFC for a reason, I've had a poor track record with off-list emails
to people that do not know me so would rather do it this way :)
Probably should have noted that I wrote the ~placeholder commit messages
though, apologies. I'll sort that out for a potential v1.

> Therefore, please double-check everybody that contributed to the lines
> you are sending (e.g. via `git-blame`) and add them as

That's what I did! Unless I missed something that was non-obvious, the
only name on the commits I lifted was you. Is there somewhere else I
should have looked for that information?

> `Co-developed-by`. Also, please add a note of what you changed/wrote
> e.g. square brackets, and `Link` tags to the original PRs/discussions.

If this goes to v1, I will note that I wrote the commit messages.

Apologies,
Conor.
Palmer Dabbelt Feb. 24, 2023, 9:31 p.m. UTC | #3
On Fri, 24 Feb 2023 05:36:08 PST (-0800), Conor Dooley wrote:
> This is a somewhat blind (and maybe foolish) attempt at enabling Rust
> for RISC-V. I've tested this on Icicle, and the modules seem to work.
> I'd like to play around with Rust on RISC-V, but I'm not interested in
> using downstream kernels, so figured I should try and see what's
> missing...
> I've tagged this as RFC in case I've missed some "WAaaaa you can't do
> this" somewhere :)

I'm fine with it, but IIRC the Rust support for most targets was pulled 
out as they weren't deemed ready to go yet.  If the Rust folks are OK 
turning on RISC-V support then it's fine with me, but I think it's 
really more up to them at this point.

So

Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

in case folks want to take it via some Rust-related tree, but I'm also 
fine taking it via the RISC-V tree if that's easier.

>
> Thanks,
> Conor.
>
> CC: Miguel Ojeda <ojeda@kernel.org>
> CC: Alex Gaynor <alex.gaynor@gmail.com>
> CC: Wedson Almeida Filho <wedsonaf@gmail.com>
> CC: Boqun Feng <boqun.feng@gmail.com>
> CC: Gary Guo <gary@garyguo.net>
> CC: Björn Roy Baron <bjorn3_gh@protonmail.com>
> CC: Jonathan Corbet <corbet@lwn.net>
> CC: Paul Walmsley <paul.walmsley@sifive.com>
> CC: Palmer Dabbelt <palmer@dabbelt.com>
> CC: Nathan Chancellor <nathan@kernel.org>
> CC: Nick Desaulniers <ndesaulniers@google.com>
> CC: Tom Rix <trix@redhat.com>
> CC: rust-for-linux@vger.kernel.org
> CC: linux-doc@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: linux-riscv@lists.infradead.org
> CC: llvm@lists.linux.dev
>
> Miguel Ojeda (2):
>   scripts: generate_rust_target: enable building on RISC-V
>   RISC-V: enable building the 64-bit kernels with rust support
>
>  Documentation/rust/arch-support.rst |  2 ++
>  arch/riscv/Kconfig                  |  1 +
>  arch/riscv/Makefile                 |  3 ++-
>  scripts/generate_rust_target.rs     | 19 +++++++++++++++++++
>  4 files changed, 24 insertions(+), 1 deletion(-)
Miguel Ojeda Feb. 24, 2023, 10:20 p.m. UTC | #4
On Fri, Feb 24, 2023 at 10:00 PM Conor Dooley <conor@kernel.org> wrote:
>
> The stuff that I have lifted here all had you as the sole author in the
> "rust" branch downstream, which is why I gave you authorship. Namely:
> afba78eacb9b ("rust: generate target specification files on the fly")
> 732b3c386328 ("rust: target: remove `cpu`")
>
> I don't see anything from [1] in these commits, so I don't think that I
> made a mistake here.

It is true that I converted the original target spec files into the
script, so I added the final lines. However, in that first commit some
of the deleted files (related to RISC-V) were created by Gary. Thus it
still feels a bit wrong to not credit Gary or even mention him.

For instance, consider an even more extreme case: somebody moving a
file or doing formatting/whitespace changes. Would they be the main
and only author?

> It's RFC for a reason, I've had a poor track record with off-list emails
> to people that do not know me so would rather do it this way :)
> Probably should have noted that I wrote the ~placeholder commit messages
> though, apologies. I'll sort that out for a potential v1.

No problem!

> That's what I did! Unless I missed something that was non-obvious, the
> only name on the commits I lifted was you. Is there somewhere else I
> should have looked for that information?

I would have traced the commits back a bit more. For instance, in the
first commit you mention above, one may see the RISC-V target files
were removed, so that means something was already there. Checking who
added those files leads to a few commits from Gary (and one from
Daniel). And then it is about making a judgement call trying to be
fair :)

Cheers,
Miguel
Conor Dooley Feb. 24, 2023, 10:32 p.m. UTC | #5
On Fri, Feb 24, 2023 at 11:20:34PM +0100, Miguel Ojeda wrote:
> On Fri, Feb 24, 2023 at 10:00 PM Conor Dooley <conor@kernel.org> wrote:

> > That's what I did! Unless I missed something that was non-obvious, the
> > only name on the commits I lifted was you. Is there somewhere else I
> > should have looked for that information?
> 
> I would have traced the commits back a bit more. For instance, in the
> first commit you mention above, one may see the RISC-V target files
> were removed, so that means something was already there. Checking who
> added those files leads to a few commits from Gary (and one from
> Daniel).

> And then it is about making a judgement call trying to be fair :)

It was a few hours ago that I looked properly, but, IIRC, it was a
conversion from config files to code? My understanding was that
config files were not copyrightable, hence I looked only at the
authorship of the code. I'm usually on the anal side about SoB stuff,
so it is not a matter of me not bothering!
Miguel Ojeda Feb. 24, 2023, 10:38 p.m. UTC | #6
On Fri, Feb 24, 2023 at 10:32 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> I'm fine with it, but IIRC the Rust support for most targets was pulled
> out as they weren't deemed ready to go yet.  If the Rust folks are OK

So we trimmed the original series from v8 to v9 as much as possible in
order to upstream things piece by piece, get maintainers involved, and
so on; i.e. they were not trimmed because they were not ready.

Having said that, for the architectures support in particular, what we
had is indeed a prototype: each architecture we added was able to
compile, boot into QEMU, load the sample Rust modules, pass a few
tests, and so on in our CI, using a couple kernel configs. But that is
just the basic support, and it does not mean it works for other kernel
configs, all hardware, all security features, and so on.

So it depends on how you want to approach it, whether you are
interested in the basic support or not, etc. In any case, I would
recommend having an expert on the architecture take a look to
double-check things look sane, run some tests on real hardware, etc.

> turning on RISC-V support then it's fine with me, but I think it's
> really more up to them at this point.
>
> So
>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> in case folks want to take it via some Rust-related tree, but I'm also
> fine taking it via the RISC-V tree if that's easier.

Thanks Palmer! We are trying to get maintainers of the different
subsystems/archs/... involved so that they maintain the different Rust
bits we are upstreaming, so ideally it would go through the RISC-V
tree.

Cheers,
Miguel
Palmer Dabbelt Feb. 24, 2023, 11:18 p.m. UTC | #7
On Fri, 24 Feb 2023 14:38:28 PST (-0800), miguel.ojeda.sandonis@gmail.com wrote:
> On Fri, Feb 24, 2023 at 10:32 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> I'm fine with it, but IIRC the Rust support for most targets was pulled
>> out as they weren't deemed ready to go yet.  If the Rust folks are OK
>
> So we trimmed the original series from v8 to v9 as much as possible in
> order to upstream things piece by piece, get maintainers involved, and
> so on; i.e. they were not trimmed because they were not ready.

OK, cool, that's way less scary.

> Having said that, for the architectures support in particular, what we
> had is indeed a prototype: each architecture we added was able to
> compile, boot into QEMU, load the sample Rust modules, pass a few
> tests, and so on in our CI, using a couple kernel configs. But that is
> just the basic support, and it does not mean it works for other kernel
> configs, all hardware, all security features, and so on.
>
> So it depends on how you want to approach it, whether you are
> interested in the basic support or not, etc. In any case, I would
> recommend having an expert on the architecture take a look to
> double-check things look sane, run some tests on real hardware, etc.

We generally take stuff pretty early in RISC-V land, for example we take 
a bunch of stuff that's just in the ISA but doesn't have any hardware 
yet.  The good news is that we don't really have any of the complicated 
language-tied features in RISC-V land, so with any luck it's pretty 
straight-forward to flip on.

>> turning on RISC-V support then it's fine with me, but I think it's
>> really more up to them at this point.
>>
>> So
>>
>> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>>
>> in case folks want to take it via some Rust-related tree, but I'm also
>> fine taking it via the RISC-V tree if that's easier.
>
> Thanks Palmer! We are trying to get maintainers of the different
> subsystems/archs/... involved so that they maintain the different Rust
> bits we are upstreaming, so ideally it would go through the RISC-V
> tree.

Works for me.

I've got a few other things in the pipeline for this merge window so 
this probably won't make it, but I'll dig in after that.  We've got a 
bunch of Rust-types floating around Rivos as well, so with any luck 
someone else will have some time to poke around.  Having a full cycle in 
linux-next is probably the right way to go for this sort of thing 
anyway, as it's likely to shake out some long-tail issues.

That'll also give us time to sort out the authorship issues, which we'd 
of course need to do before merging anything.
Miguel Ojeda Feb. 25, 2023, 8:20 a.m. UTC | #8
On Fri, Feb 24, 2023 at 11:32 PM Conor Dooley <conor@kernel.org> wrote:
>
> It was a few hours ago that I looked properly, but, IIRC, it was a
> conversion from config files to code? My understanding was that
> config files were not copyrightable, hence I looked only at the
> authorship of the code. I'm usually on the anal side about SoB stuff,
> so it is not a matter of me not bothering!

It is not so much about copyright, but about crediting people fairly.
Gary has done a significant amount of the work that eventually became
part of the patches here, and I would have sent the patch with at
least a `Co-developed-by` from him.

Not to mention every commit in the kernel has a SoB, regardless of
what files are modified -- I would not remove a SoB from somebody else
when forwarding a patch even if they were all non-copyrightable files.

Cheers,
Miguel
Miguel Ojeda Feb. 25, 2023, 8:37 a.m. UTC | #9
On Sat, Feb 25, 2023 at 12:18 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> Works for me.
>
> I've got a few other things in the pipeline for this merge window so
> this probably won't make it, but I'll dig in after that.  We've got a
> bunch of Rust-types floating around Rivos as well, so with any luck
> someone else will have some time to poke around.  Having a full cycle in
> linux-next is probably the right way to go for this sort of thing
> anyway, as it's likely to shake out some long-tail issues.

Thanks a lot! That would be great.

At least from our side, no rush. In fact, we are letting users (or
arch maintainers) to request/submit the architectures themselves as
they need/want them.

Cheers,
Miguel
Palmer Dabbelt March 6, 2023, 7:18 p.m. UTC | #10
On Sat, 25 Feb 2023 00:37:29 PST (-0800), miguel.ojeda.sandonis@gmail.com wrote:
> On Sat, Feb 25, 2023 at 12:18 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> Works for me.
>>
>> I've got a few other things in the pipeline for this merge window so
>> this probably won't make it, but I'll dig in after that.  We've got a
>> bunch of Rust-types floating around Rivos as well, so with any luck
>> someone else will have some time to poke around.  Having a full cycle in
>> linux-next is probably the right way to go for this sort of thing
>> anyway, as it's likely to shake out some long-tail issues.
>
> Thanks a lot! That would be great.
>
> At least from our side, no rush. In fact, we are letting users (or
> arch maintainers) to request/submit the architectures themselves as
> they need/want them.

It's time for the next release.  IIUC there were some authorship issues 
here, did you guys want to re-spin this with those sorted out?  I can 
give it a shot if you want, but I'm probably as likely to screw it up as 
anyone else...
Conor Dooley March 6, 2023, 7:26 p.m. UTC | #11
On Mon, Mar 06, 2023 at 11:18:06AM -0800, Palmer Dabbelt wrote:
> On Sat, 25 Feb 2023 00:37:29 PST (-0800), miguel.ojeda.sandonis@gmail.com wrote:
> > On Sat, Feb 25, 2023 at 12:18 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > 
> > > Works for me.
> > > 
> > > I've got a few other things in the pipeline for this merge window so
> > > this probably won't make it, but I'll dig in after that.  We've got a
> > > bunch of Rust-types floating around Rivos as well, so with any luck
> > > someone else will have some time to poke around.  Having a full cycle in
> > > linux-next is probably the right way to go for this sort of thing
> > > anyway, as it's likely to shake out some long-tail issues.
> > 
> > Thanks a lot! That would be great.
> > 
> > At least from our side, no rush. In fact, we are letting users (or
> > arch maintainers) to request/submit the architectures themselves as
> > they need/want them.
> 
> It's time for the next release.  IIUC there were some authorship issues
> here, did you guys want to re-spin this with those sorted out?  I can give
> it a shot if you want, but I'm probably as likely to screw it up as anyone
> else...

It's in my queue to respin, I wasn't bothered doing one until after -rc1.
I'll try to get to it tomorrow since it's simpler than anything else I
currently owe one for.
Should be able to do something that satisfies everyone, or, at least,
gives everyone the opportunity to be satisfied!

Cheers,
Conor.
Miguel Ojeda March 6, 2023, 7:28 p.m. UTC | #12
On Mon, Mar 6, 2023 at 8:18 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> It's time for the next release.  IIUC there were some authorship issues
> here, did you guys want to re-spin this with those sorted out?  I can
> give it a shot if you want, but I'm probably as likely to screw it up as
> anyone else...

No problem on my side, and either way is fine! Thanks!

I think what is important is that Gary is aware and agrees.

Gary: do you prefer to be the main author or the `Co-developed-by` author here?

Cheers,
Miguel
Gary Guo March 7, 2023, 2:20 a.m. UTC | #13
On Mon, 6 Mar 2023 20:28:47 +0100
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:

> On Mon, Mar 6, 2023 at 8:18 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > It's time for the next release.  IIUC there were some authorship issues
> > here, did you guys want to re-spin this with those sorted out?  I can
> > give it a shot if you want, but I'm probably as likely to screw it up as
> > anyone else...  
> 
> No problem on my side, and either way is fine! Thanks!
> 
> I think what is important is that Gary is aware and agrees.
> 
> Gary: do you prefer to be the main author or the `Co-developed-by` author here?

Co-developed-by is fine.

Best,
Gary