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.

  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...

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


Reply via email to