| Message ID | 20230224133609.2877396-1-conor.dooley@microchip.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RISC-V: enable rust | expand |
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
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.
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(-)
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
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!
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
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.
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
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
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...
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.
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
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