Re: [PATCH v2 00/20] Add Multi-Core Debug (MCD) API support

[Markus Armbruster <armbru@redhat.com>]

Mario Fleischmann <mario.fleischmann@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.

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.

[...]