riscv-non-isa / riscv-semihosting

https://jira.riscv.org/browse/RVG-39
Creative Commons Attribution Share Alike 4.0 International
27 stars 8 forks source link

Does it make sense to implement this specification? #9

Closed arichardson closed 5 months ago

arichardson commented 10 months ago

I was looking into implementing semihosting support in a simulator and noticed that this current document does not give me the required information on which calls need to be supported. QEMU (and possibly other simulators) as well as some baremetal libraries such as picolibc implement this spec already, but I see it's now listed as a discussion document. Does it make sense to add semihosting support to other simulators?

Previous versions of the document included a link to the Arm semihosting specification which lists all the calls. Simply removing the link in https://github.com/riscv-software-src/riscv-semihosting/commit/f2e0eaf46d34a54ebbc1799c9a8879c4cc544fe4 means it cannot be implemented. Could link to the QEMU implementation instead?

keith-packard commented 10 months ago

I'm mystified as to why the link to the ARM specification was removed. Adding text that duplicates the ARM text into the risc-v spec would only be a partial solution; the goal was to make it clear that any implementation could (and should) be shared between the two targets, hence having an explicit link to the relevant ARM document.

cmuellner commented 10 months ago

I'm mystified as to why the link to the ARM specification was removed. Adding text that duplicates the ARM text into the risc-v spec would only be a partial solution; the goal was to make it clear that any implementation could (and should) be shared between the two targets, hence having an explicit link to the relevant ARM document.

There are several issues with referencing the main branch of an external repository:

Removing the link might look like a drastic step, but given that nobody has cared about the spec since then, it was the right decision.

Probably it would be better to cite a defined (release) version of an external specification after ensuring that the reference will not cause legal issues. Further, a list of parts in that cited external document that are relevant in the referencing document is most likely needed (maybe only some parts are mandatory and others are optional). But there is also the need for a justification to include an external specification as the main part of a new specification (even if obvious such fundamental decisions of the design of a specification should be stated explicitly).

Further, there should be some active interest in ensuring that implementations of that spec exist and are in sync with the spec. For the toolchain specs we make sure that at least Binutils/GCC/LLVM have patches before we merge changes. OpenSBI has a similar mechanism. Something similar could be done here as well.

Another question to answer is if this document should become a ratified specification or will remain a community project (no ratification). This question should be discussed with the technical leadership of RVI.

Getting this specification in a maintainable state should not be a big challenge, but it requires someone who can invest the necessary time plus a community that helps with contributions and reviews. If there are volunteers I can help to get the ball rolling.

arichardson commented 10 months ago

I'm mystified as to why the link to the ARM specification was removed. Adding text that duplicates the ARM text into the risc-v spec would only be a partial solution; the goal was to make it clear that any implementation could (and should) be shared between the two targets, hence having an explicit link to the relevant ARM document.

There are several issues with referencing the main branch of an external repository:

  • Every technical change in that external document is immediately also mandatory in your specification (you lose your versioning).
  • Every non-compatible change in the external document gives you no room for additional backward compatibility.
  • Every license change in the external doc might cause legal troubles with your specification.

Agreed that all of these are a problem, but they are all the same underlying problem, namely that the target could change in incompatible ways. If the link is updated to a versioned release (e.g. the 2023Q3 release: https://github.com/ARM-software/abi-aa/blob/2023Q3/semihosting/semihosting.rst that problem goes away?

cmuellner commented 10 months ago

I'm mystified as to why the link to the ARM specification was removed. Adding text that duplicates the ARM text into the risc-v spec would only be a partial solution; the goal was to make it clear that any implementation could (and should) be shared between the two targets, hence having an explicit link to the relevant ARM document.

There are several issues with referencing the main branch of an external repository:

  • Every technical change in that external document is immediately also mandatory in your specification (you lose your versioning).
  • Every non-compatible change in the external document gives you no room for additional backward compatibility.
  • Every license change in the external doc might cause legal troubles with your specification.

Agreed that all of these are a problem, but they are all the same underlying problem, namely that the target could change in incompatible ways. If the link is updated to a versioned release (e.g. the 2023Q3 release: https://github.com/ARM-software/abi-aa/blob/2023Q3/semihosting/semihosting.rst that problem goes away?

Yes, that's what I meant with

Probably it would be better to cite a defined (release) version of an external specification after ensuring that the reference will not cause legal issues.

avpatel commented 5 months ago

I have created new v0.3 draft of this specification with proper citation to ARM semihosting and separate introduction section.

https://github.com/riscv-non-isa/riscv-semihosting/releases/tag/0.3

arichardson commented 5 months ago

Thank you, the new wording addresses this issue.