On Fri, Mar 25, 2022 at 6:53 PM Jukka Laitinen <jukka.laiti...@iki.fi> wrote:
> Hi, > > I was trying to make a more general statement than starting discussion > on separate PRs, but let me shortly answer still > > On 25.3.2022 10.32, Xiang Xiao wrote: > > On Fri, Mar 25, 2022 at 3:35 PM Jukka Laitinen <jukka.laiti...@iki.fi> > > wrote: > > > >> Hi, > >> > >> > >> As an another example, we would very much like to bring in > >> CONFIG_BUILD_KERNEL support for RISC-V for NuttX, as we have worked hard > >> on this for some time, and have it working. Now, even when this work it > >> is only additions, not breaking anything for FLAT_BUILD users, > > > > Yes, it doesn't break FLAT_BUILD, but doesn't mean KERNEL design or code > is > > perfect, that's why the interesting people can give the comment. > But of course! > >> is stuck > >> in endless debate where half of the comments show that the reviewer > >> doesn't know the RISC-V ISA thoroughly, > >> > >> > >> Sorry, I give you this impression, but I have worked on RiSCV since > 2018:(. > > I wasn't referring to you with the above comment... and I am sorry about > this. I'll try to avoid anything that can be interpreted as comments to > person, in the future. > > >> and is referring to how things > >> are done in ARM world. And the other half is largely useless style > >> nitpicking. > >> > > I think the most debate is about how S-mode talks with M-mode in this PR. > > The module design and standard compliant is always NuttX core value and > > highlight in: > > https://github.com/apache/incubator-nuttx/blob/master/INVIOLABLES.md > > Since OpenSBI is adopted by many OS to interact with M-mode firmware: > > https://github.com/riscv-software-src/opensbi > > It's always good to follow the best practice and what I insist in this > PR: > > we can implement a minimal subset of OpenSBI to support S-mode NuttX, > > We (TII, it's research partners and subcontractors) will not be using > OpenSBI for interfacing between S-mode and M-mode inside NuttX, since it > doesn't make any sense for us. For running NuttX in kernel mode, we will > not "implement a subset of OpenSBI". We (Ville) have implemented the > "native SBI for NuttX". OpenSBI is not a standard, it is just a library > maintained by a few people. It is much better to just do what is needed > to do what is good for NuttX, following the standard RISC-V ISA spec, > and not blindly go with a 3rd party bloated library. > Ok, if you still prefer native SBI, please at least add a choice option in Kconfig, so other people can support OpenSBI later. > > > but > > don't invent a private interface since M-mode firmware mayn't run NuttX > at > > all. > > Now this is fair request, and I don't see any problems in implementing > this also for the mpfs in the future. Currently that PR (IMHO) doesn't > prevent having another M-mode firmware+any custom SBI such as OpenSBI on > other platforms. Currently there are also parts under mpfs board, which > can be generalized later on to avoid code copy paste, if others want to > use our implementation on other risc-v archs. Right now there is no > copy-paste code added, since this is not used in any other risc-v archs. > > I see bringing in CONFIG_BUILD_KERNEL as a very important step, and I > see it a huge step forward if we can get in even one new architecture in > supporting that. But putting too much pressure on making world perfect > for the whole risc-v/nuttx community at once is just too much. > Incremental steps forward is much better way... First make it work on > one board, then generalize functionality when taking it into use in > another and so on... > Sure, but since it's a bigger change and the implementation is unique in many places, it's expected that the review process will be a bit longer. > > Anyhow, let's try to keep the discussion regarding a single PR within > that PR in github. And I am really sorry if you felt that I was accusing > you of something. it was not my intention. I truly appreciate that you > are putting so much time and effort for reviewing patches to NuttX. > > Thanks, > > Jukka > > >
