On 20.05.2025 09:36, Markus Armbruster wrote: > Mario Fleischmann <mario.fleischm...@lauterbach.com> writes: > >> This patch series introduces support for the Multi-Core Debug (MCD) API, a >> commonly used debug interface by emulators. The MCD API, defined through a >> header file, consists of 54 functions for implementing debug and trace. >> However, since it is a header-file-only interface, MCD does not specify a >> communication protocol. >> >> To keep the overhead of a communication protocol on top of MCD minimal, >> we follow a remote procedure call approach by using QAPI as an interface >> definition and transport infrastructure. This way, we can use qapi-gen to >> take care of generating the infrastructure to dispatch MCD functions and >> to (un)marshal their arguments and results. Furthermore, qapi-doc and qtest >> provide good integration into QEMU's documentation and test frameworks. >> >> In v1 of this patch series, the MCD protocol was directly integrated in QMP >> and the QMP monitor was responsible for dispatching MCD's server stub. This >> introduced a dependency between QEMU's machine protocol and the MCD debug >> protocol which is not to be expected. For this reason, v2 introduces a MCD >> monitor which uses as much of the QMP monitor's framework as possible but >> keeps the two protocols separate from each other. >> Similarly, MCD's test suite uses as much of the qtest framework as is useful >> for sending JSON commands to the QEMU under test but adds new code where >> required to prevent dependencies to QMP. >> >> To enable MCD, configure QEMU with `--enable-mcd`. >> >> To start the MCD monitor, run QEMU with the `-mcd` option: >> qemu-system-<arch> [options] -qmp tcp::1235,server=on,wait=off >> >> To run the MCD test suite independently, start `mcd-test`: >> V=1 QTEST_QEMU_BINARY="./qemu-system-<arch> [options]" tests/qtest/mcd-test >> >> To connect from a MCD client, a client stub corresponding to this >> patch series can be found at https://gitlab.com/lauterbach/mcdrefsrv > > I'm okay with the general approach. > > [...] > >> MAINTAINERS | 9 + >> docs/interop/index.rst | 1 + >> docs/interop/mcd.rst | 65 + >> gdbstub/gdbstub.c | 15 +- >> include/exec/gdbstub.h | 18 +- >> include/exec/mcdstub.h | 18 + >> mcd/mcd_api.h | 3963 +++++++++++++++++++++++++++++++++ >> mcd/mcd_monitor.c | 90 + >> mcd/mcd_qapi.c | 505 +++++ >> mcd/mcd_qapi.h | 81 + >> mcd/mcd_server.c | 2274 +++++++++++++++++++ >> mcd/mcd_stub.c | 988 ++++++++ >> mcd/meson.build | 60 + >> meson.build | 5 + >> meson_options.txt | 3 + >> qapi/mcd.json | 2366 ++++++++++++++++++++ > > The schema is too big for me to review in detail. I understand it's > designed to mirror mcd/mcd_api.h closely, so review would be limited to > checking the way you mirror is sane. I doubt that would be a good use > of my time, but if you'd like advice on any non-trivial parts, just ask.
Let's do it this way: As explained in "Re: [PATCH v2 07/20] mcd: Implement target initialization API" <a71a747f-d8c3-40ac-ab61-0628dffbb...@lauterbach.com> there are reasons to deviate from mcd/mcd_api.h in mcd/qapi-schema.json. In v3, I can highlight those changes briefly in the relevant commit messages. That way, you have the option to focus on the non-trivial parts. > Some formatting nits caught my eye. docs/devel/qapi-code-gen.rst > section "Documentation markup": > > For legibility, wrap text paragraphs so every line is at most 70 > characters long. > > Separate sentences with two spaces. > > I recommend to read the entire section. Will do!
