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
