Thanks for taking the time to write this up! On Wed, 19 Jan 2022 at 02:30, Alistair Francis <alistai...@gmail.com> wrote: > > On Wed, Jan 19, 2022 at 11:19 AM Alistair Francis <alistai...@gmail.com> > wrote: > > > > On Wed, Jan 19, 2022 at 9:22 AM Philipp Tomsich > > <philipp.toms...@vrull.eu> wrote: > > > > > > Alistair, > > > > > > Some of us (the merit almost exclusively goes to Kito) have been > > > working towards a similar policy for GCC/binutils and LLVM. > > > This currently lives in: > > > https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17 > > > > Ah cool! We can use that as a good starting point. > > > > > > > > A few comments & a question below. > > > > > > Thanks, > > > Philipp. > > > > > > On Tue, 18 Jan 2022 at 23:53, Alistair Francis <alistai...@gmail.com> > > > wrote: > > > > > > > > On Fri, Jan 14, 2022 at 6:22 AM Philipp Tomsich > > > > <philipp.toms...@vrull.eu> wrote: > > > > > > > > > > This adds the decoder and translation for the XVentanaCondOps custom > > > > > extension (vendor-defined by Ventana Micro Systems), which is > > > > > documented at > > > > > https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf > > > > > > > > > > This commit then also adds a guard-function (has_XVentanaCondOps_p) > > > > > and the decoder function to the table of decoders, enabling the > > > > > support for the XVentanaCondOps extension. > > > > > > > > > > Signed-off-by: Philipp Tomsich <philipp.toms...@vrull.eu> > > > > > > > > This looks reasonable to me. > > > > > > > > I'm going to leave this for a bit in case there are any more comments. > > > > > > > > I was a little worried that taking vendor extensions isn't the right > > > > move, as we might get stuck with a large number of them. But this is > > > > pretty self contained and I think with the growing RISC-V interest > > > > it's something we will eventually need to support. > > > > > > > > I'm going to update the QEMU RISC-V wiki page with this to make the > > > > position clear (comments very welcome) > > > > > > > > === RISC-V Extensions === > > > > As RISC-V has a range of possible extensions, QEMU has guidelines for > > > > supporting them all. > > > > > > > > If an extension is frozen or ratified by the RISC-V foundation, it can > > > > be supported in QEMU. > > > > > > > > If an official RISC-V foundation extension is in a reasonable draft > > > > state, that is not too many changes are still expected, it can be > > > > supported experimentally by QEMU. Experimental support means it must > > > > be disabled by default and marked with a "x-" in the properties. QEMU > > > > will only support the latest version of patches submitted for a draft > > > > extension. A draft extension can also be removed at any time if it > > > > conflicts with other extensions. > > > > > > > > QEMU will also support vendor extensions. Vendor extensions must be > > > > disabled by default, but can be enabled for specific vendor CPUs and > > > > boards. Vendor extensions must be maintained and tested by the vendor. > > > > > > I guess I should create a v3 with appropriate paths in the MAINTAINERS > > > file? > > > > Hmm... Good point. I don't think you have to if you don't want to. > > > > My point here was more to just make it clear that upstream QEMU is not > > a dumping ground for vendor extensions to get them maintained by > > someone else. Obviously we won't purposely break things just for fun. > > There is an expectation that the vendor tests their extensions and > > responds to bug reports and things like that. > > > > > > > > > Vendor extensions can not interfere with other extensions and can not > > > > be obtrusive to the RISC-V target code. > > > > > > I know that there is some interest to have the XtheadV (the > > > instructions previously known as vectors 0.7.1-draft) supported and we > > > have the reality of a deployed base that implements it in hardware. > > > This would conflict with the opcode space used by the standard RISC-V > > > vectors, so it makes for an interesting test case (even if just to > > > clarify our intent)... > > > Personally, I would like to avoid precluding inclusion of something > > > useful (of course, "Vendor extensions must be maintained and tested by > > > the vendor." has to apply!), if a vendor was going to step up and also > > > offers to maintain it. > > > > Yeah... this is unfortunate. I agree that having the 0.7.1-draft > > extensions supported would be great. There is hardware that supports > > it. > > > > I think this point still stands though. IF the XtheadV implementation > > is self contained and doesn't interfere with the vector extensions, > > then that's great and we can support it. If instead it adds a large > > amount of conditionals to the released vector extension code then I > > don't think we can take it. > > > > There is some wiggle room, but the RISC-V tree already has enough > > going on and very little reviewers. If in the future we get more > > reviewers and testers we can re-evaulate what is acceptable, but for > > now I think we need to be a little strict. (Hint to any companies to > > give developers time to review) > > > > > > > > So let's assume such a (very significant) addition were factored out > > > similarly, interfacing just through a hook in decode_op and > > > argument-parsing logic that ensures that the conflicting > > > standard-extension is turned off: would this still be acceptable under > > > this policy — or would it trip the "obtrusive" condition? > > > > I think that would be acceptable, I wouldn't say that is obtrusive as > > it's self contained. > > Ok, take two: > > === RISC-V Foundation Extensions === > As RISC-V has a range of possible extensions, QEMU has guidelines for > supporting them all. > > If an extension is frozen or ratified by the RISC-V foundation, it can > be supported in QEMU. Generally we will try to support as many versions > as feasible, following the usual QEMU deprecation policy to remove old > versions. > > If an official RISC-V foundation extension is in a reasonable draft > state, that is not too many changes are still expected, it can be > supported experimentally by QEMU. Experimental support means it must > be disabled by default and marked with a "x-" in the CPU/board properties. > Draft extensions can be enabled by specific CPUs or boards if the hardware > supports that extension.
Should we include a version number on experimental versions? LLVM requires users to fully specify the version, when using experimental versions. This may be a useful stereotype also for QEmu, as it ensures that users are aware that the underlying specification version has changed (e.g., when someone requests x-zbb-0p92 and our implementation moves to x-zbb-0p93 (there was a difference in encoding of the minimum/maximum operations in-between)), an error will be raised early instead of having a computation go wrong later. > QEMU will only support the latest version of patches submitted for a draft > extension. A draft extension can also be removed at any time and does not > follow QEMU's deprecation policy. > > === RISC-V Custom Extensions/Instructions === > Support for custom instruction set extensions are an important part of RISC-V, > with large encoding spaces reserved of vendor extensions. > > QEMU follows similar rules to the RISC-V toolchain convention, as described > here: https://github.com/riscv-non-isa/riscv-toolchain-conventions/pull/17 > > QEMU will support vendor extensions. Vendor extensions must be > disabled by default, but can be enabled for specific vendor CPUs and > boards. The vendor extensions should use prefixes and names as described in > https://github.com/riscv-non-isa/riscv-toolchain-conventions > > Vendor extensions must be maintained and tested by the vendor. Upstream will > take efforts to not break extensions, but testing and bug fixes should be > done by the vendor. Patches to add support for open source toolchains are > unlikely to be accepted without specification documents being made available > publicly. > > Vendor extensions can not interfere with other extensions and can not > be obtrusive to the core RISC-V target code. > > If you are looking to add support for vendor extensions, it is recommended > that you get involved in the QEMU review process. It is also recommended that > you send your patches as early as possible to get community feedback before > they are fully implemented. This is especially important if you are modifying > core code. > > Alistair
