On Mon, Sep 6, 2021 at 4:49 PM Ruinland ChuanTzu Tsai <ruinl...@andestech.com> wrote: > > > Hi Alistair, > > Thanks for the heads up about the upcoming unification of RISC-V 32/64 > targets. > Yet I have several concerns and would like to have some brainstorming > regarding > such topics - -
No worries, I'm happy to discuss. > > That is, could you elaborate more about the "runtime check/switch" which you > mentioned in the previous e-mail : > https://lists.nongnu.org/archive/html/qemu-devel/2021-08/msg02154.html > I'm not quite following the context. Yep, so something along the lines of this in `riscv_csrrw()` if (cpu == "MyCustomCPU") { my_custom_csr[csrno].read(); } So we check if using the CPU then apply extra CSR accesses. > If we don't have a way to toggle which (vendor) cores, which will be used, > during compile time, it means that we have to build all the vendor code and > link them all together; and we might have the chance to encounter collision on > csrno between different vendors. I don't see how they will collide as we will only act on 1, based on the CPU we are using. > > Secondly, I'm not quite sure about how we're going to merge decode tree files > across RV32 and RV64. Vendor-designed custom instruction would have a > different > encoding scheme on bitfields for RV32 and RV64. Currently, we (Andes) are > using > different decodetree sources for gen32 and gen64 in > `target/riscv/meson.build`. Ok, so custom instructions are a whole different problem. I think we should leave that for now and focus on CSRs. A quick look though and I suspect we could do a similar CPU check in decode_opc(). Dealing with the decodetree will be problematic though. > I'm preparing the patch to demonstrate such hiccups. > > As far as I know, there's no control flow logic for decodetree to parse > decodetree files differently. (e.g. ifdef XLEN == 32 then blah, blah). > > To meet in the halfway, maybe after the grand unification on RV32/64, we can > still confine vendor custom logic (i.e. instrucions and CSRs) to be toggled by > whether a certain vendor cpu model is selected ? I honestly don't see a scenario where that happens. The maintenance overhead and confusion of changing the CPUs at build time is too high. I also don't think we should need that for CSR accesses. Custom instructions are a whole different can of worms. > > By the way, I'm wondering how our friends from T-Head (Guo Ren ?) regard this > issue ? AFAIK, they forked QEMU from v3.2.0 and applied their vendor features > on top of it for quite a while. I'm not sure. Alistair > > Cordially yours, > Ruinland ChuanTzu Tsai > > On Thu, Sep 02, 2021 at 12:25:20PM +1000, Alistair Francis wrote: > > On Fri, Aug 27, 2021 at 1:16 AM Ruinland Chuan-Tzu Tsai > > <ruinl...@andestech.com> wrote: > > > > > > From: Ruinland ChuanTzu Tsai <ruinl...@andestech.com> > > > > > > During my modification on my previous patch series for custom CSR > > > support, I > > > believe this issue deserves its own discussion (or debate) because it's > > > _not_ > > > as simple as "just put those options in Kconfig". > > > > > > The obstables I've encountered and the kluges I came up is listed as > > > follow : > > > > > > (1) Due to the design of top-level meson.build, all Kconfig options will > > > land > > > into `*-config-devices.h` since minikconf will be only used after > > > config_target > > > being processed. This will let to the fact that linux-users won't be able > > > to > > > use custom CSR code properly becuase they only includes > > > `*-config-devices.h`. > > > And that is reasonble due to the fact that changes on cpu.c and csr.c is a > > > target-related matter and linux-user mode shouldn't include device related > > > headers in most of cases. > > > > > > So, modify meson.build to parse target/riscv/Kconfig during config_target > > > phase > > > is without doubts necessary. > > > > > > (2) Kconfig option `RISCV_CUSTOM_CSR` is introduced for RISC-V cpu models > > > to > > > toggle it at its will. Yet due to the fact that csr.o and cpu.o are linked > > > altogether for all CPU models, the suffer will be shared without option. > > > The only reasonable way to seperate build the fire lane which seperates > > > vendor > > > flavored cpu and spec-conformed ones, is to build them seperately with > > > options > > > toggled diffrently, just like RV32 and RV64 shares almost the same source > > > base, > > > yet the sources are compiled with differnt flags/definitions. > > > > > > To achieve that, miraculously, we can just put *.mak files into `target` > > > directoy, because that's how `configure` enumerates what targets are > > > supported. > > > > > > (3) The longest days are not over yet, if we take a good look at how the > > > minikconf > > > is invoked during config_devices and in what way *.mak presented its > > > options > > > inside `default-configs/devices`, we can see that *.mak files there is > > > formated > > > in `CONFIG_*` style and the minikconf is reading directly during > > > config_device > > > phase. That's totally different from *.mak files presented in > > > `default-configs/targets`. To make the parsing logic consistent, I > > > introduce a rv_custom directory inside which contains minikconf-parsable > > > mak files. > > > > > > With this patches, ones can build a A25/AX25 linux-user platform by : > > > $ ./configure > > > --target-list=riscv64-andes-linux-user,riscv32-andes-linux-user > > > > Hey! Thanks for the patches > > > > I'm not convinced that we want this though. > > > > Right now we are trying to head towards a riscv64-softmmu binary being > > able to run all RISC-V code. That include 32-bit cpus > > (qemu-riscv64-softmmu -cpu r32...) and 64-bit CPUs. We shouldn't be > > splitting out more targets. > > > > It also goes against the general idea of RISC-V in that everyone has a > > standard compliant implementation, they can then add extra > > functionality. > > > > In terms of Kconfig options. It doesn't seem like a bad idea to have > > an option to fully disable custom CSRs. That way if someone really > > wants performance and doesn't want custom CSRs they can disable the > > switch. Otherwise we leave it on and all custom CSRs are available in > > the build and then controlled by the CPU selection at runtime. If that > > ends up being too difficult to implement though then we don't have to > > have it. > > > > Thanks again for working on this. > > > > Alistair > > > > > > > $ make > > > > > > P.S. The pacthes from : > > > https://lists.gnu.org/archive/html/qemu-devel/2021-08/msg00913.html > > > is needed. A clean-up and modified version will be sent out soon. > > > > > > P.P.S. > > > I know these parts won't be easy to digest, and the further iterations > > > will be > > > needed, so I didn't ask my colleagues to sign-off for now. > > > > > > Cordially yours, > > > Ruinland ChuanTzu Tsai > > > > > > Ruinland ChuanTzu Tsai (2): > > > Adding Kconfig options for custom CSR support and Andes CPU model > > > Adding necessary files for Andes platforms, cores to enable custom CSR > > > support > > > > > > Kconfig | 1 + > > > .../devices/riscv32-andes-softmmu.mak | 17 ++++++++++++ > > > .../devices/riscv64-andes-softmmu.mak | 17 ++++++++++++ > > > .../targets/riscv32-andes-linux-user.mak | 1 + > > > .../targets/riscv32-andes-softmmu.mak | 1 + > > > .../targets/riscv64-andes-linux-user.mak | 1 + > > > .../targets/riscv64-andes-softmmu.mak | 1 + > > > .../targets/rv_custom/no_custom.mak | 0 > > > .../rv_custom/riscv32-andes-linux-user.mak | 1 + > > > .../rv_custom/riscv32-andes-softmmu.mak | 1 + > > > .../targets/rv_custom/riscv32-linux-user.mak | 1 + > > > .../targets/rv_custom/riscv32-softmmu.mak | 1 + > > > .../rv_custom/riscv64-andes-linux-user.mak | 1 + > > > .../rv_custom/riscv64-andes-softmmu.mak | 1 + > > > .../targets/rv_custom/riscv64-linux-user.mak | 1 + > > > .../targets/rv_custom/riscv64-softmmu.mak | 1 + > > > meson.build | 26 +++++++++++++++++++ > > > target/riscv/Kconfig | 6 +++++ > > > 18 files changed, 79 insertions(+) > > > create mode 100644 default-configs/devices/riscv32-andes-softmmu.mak > > > create mode 100644 default-configs/devices/riscv64-andes-softmmu.mak > > > create mode 120000 default-configs/targets/riscv32-andes-linux-user.mak > > > create mode 120000 default-configs/targets/riscv32-andes-softmmu.mak > > > create mode 120000 default-configs/targets/riscv64-andes-linux-user.mak > > > create mode 120000 default-configs/targets/riscv64-andes-softmmu.mak > > > create mode 100644 default-configs/targets/rv_custom/no_custom.mak > > > create mode 100644 > > > default-configs/targets/rv_custom/riscv32-andes-linux-user.mak > > > create mode 100644 > > > default-configs/targets/rv_custom/riscv32-andes-softmmu.mak > > > create mode 120000 > > > default-configs/targets/rv_custom/riscv32-linux-user.mak > > > create mode 120000 default-configs/targets/rv_custom/riscv32-softmmu.mak > > > create mode 100644 > > > default-configs/targets/rv_custom/riscv64-andes-linux-user.mak > > > create mode 100644 > > > default-configs/targets/rv_custom/riscv64-andes-softmmu.mak > > > create mode 120000 > > > default-configs/targets/rv_custom/riscv64-linux-user.mak > > > create mode 120000 default-configs/targets/rv_custom/riscv64-softmmu.mak > > > create mode 100644 target/riscv/Kconfig > > > > > > -- > > > 2.32.0 > > >
