Apologies for the line wrapping in yesterday's answer. Should be fixed now.
On 08.04.2025 09:00, Markus Armbruster wrote: > Mario Fleischmann <mario.fleischm...@lauterbach.com> writes: > >> Thanks a lot for the response, I really appreciate your time. >> >> On 07.04.2025 14:33, 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. We get around this limitation by following a remote >>>> procedure call approach using QMP. The client stub corresponding to this >>>> implementation can be found at https://gitlab.com/lauterbach/mcdrefsrv >>>> >>>> This series is the successor to: >>>> "[PATCH v5 00/18] first version of mcdstub" >>>> (https://patchew.org/QEMU/20231220162555.19545-1-nicolas.e...@lauterbach.com/) >>>> >>>> * Architecture-independent MCD implementation >>>> * QMP instead of custom TCP protocol >>> >>> Rationale? There must be pros and cons. >> >> Assuming you're referring to the protocol of the previous patch series: >> The previous TCP protocol only supported a subset of MCD. As the >> implementation progresses, the protocol eventually needs to be extended, >> possibly resulting in backwards compatibility problems. >> Following an RPC approach and keeping the communication layer as close >> to the MCD API as possible results in a larger protocol at first, but >> does not need to be changed afterwards. >> By directly mapping MCD functions onto QMP commands, the complexity in >> the server and client stubs can be minimized. >> >> Assuming you're referring to the QMP choice: >> QMP is being described as the "protocol which allows applications to >> control a QEMU instance". >> It provides a RPC framework which automatically (de)serializes methods >> and their parameters, even inside QTests. >> The whole interface is automatically documented. > > Let's see whether I understand. > > MCD is an established C interface. > > Your goal is to provide remote MCD for QEMU, i.e. the client uses the > MCD C interface, and the interface's implementation talks to an MCD > server integrated into QEMU via some remote transport. > > The previous version connects the two with a bespoke protocol via TCP. > The client software translates between the C interface and this > protocol. QEMU implements the protocol's server side. Designing and > maintaining a protocol is expensive. > > This versions makes two changes: > > 1. Instead of layering a protocol on top of MCD, you use MCD directly. > This eliminates protocol design and maintenance. Moreover, translation > becomes straightforward marshaling / unmarshaling for the transport. > > 2. You use QMP as a transport. This gets you marshaling / unmarshaling > for free. It also provides some useful infrastructure for tests, > documentation and such. > > Fair? Couldn't have put it better myself. >>> How much data would you expect to flow in practical usage? QMP isn't >>> designed for bulk transfer... >> >> According to ifstat, the expected data rate in practical usage is around >> >> KB/s in KB/s out >> 100 100 >> >> I fully understand your concern and agree that a JSON-based >> protocol does not result in the lowest data rate. >> >> If the data rate is the highest priority: *Before* the QMP supported was >> implemented, the MCD interface was built on a custom RPC framework, >> generated with the code generator at: >> >> https://gitlab.com/lauterbach/mcdrefsrv/-/tree/main/codegen >> >> The resulting header file was basically a set of functions capable of >> serializing MCD's function arguments into a byte stream and vice-versa: >> >> https://gitlab.com/lauterbach/mcdrefsrv/-/blob/df754cef7f19ece2d00b6ce4e307ba37e91e5dcb/include/mcd_rpc.h >> >> The QMP support was added because of the advantages listed above and in >> order to evade yet another custom communication protocol. >> As a user of the MCD interface, I haven't noticed any negative impact of >> the increased data rate in realistic debugging scenarios, even when >> trying to drive the data rate up. If that would have been the case, I >> would have sent this patch request with our custom RPC protocol. > > I see. > >>>> qemu-system-<arch> [options] -qmp tcp::1235,server=on,wait=off >>>> >>>> * Architecture-independent QTest test suite >>>> >>>> V=1 QTEST_QEMU_BINARY="./qemu-system-<arch> [options]" tests/qtest/mcd-test >>>> >>>> * Architecture-specific tests can be found at the client stub >>> >>> [...] >>> >>>> qapi/mcd.json | 2366 ++++++++++++++++++++++ >>> >>> This is *massive*. By non-blank, non-comment lines, it's the second >>> largest module in qapi/, almost 9% of the entire schema. It's larger >>> than the entire QEMU guest agent QAPI schema. The QAPI generator >>> generates some 280KiB of C code for it. >> >> I understand your point and I think it touches on the point made above >> regarding MCD's complexity: >> >>> mcd/mcd_api.h | 3963 +++++++++++++++++++++++++++++++++++++ > > Uh, that's a big one. > > Out of curiosity, what's the size of the previous version's code to > translate between the C interface and TCP? The previous version's protocol was very similar to GDB's remote serial protocol which is why the size of its implementation is comparable to that of gdbstub.c > debug/mcdstub/mcdstub.c | 2481 ++++++++++++++++++++++ Note that this file contains both the transport layer as well as the implementation and does not implement mcd_api.h which makes the two patch sets difficult to compare. >> I hope that we agree that RPC is generally the right approach to >> implement MCD. As far as the implementation is concerned, I'm open to >> any suggestion you have. I've always avoided to introduce any >> unnecessary external dependencies. > > I think you're much better qualified to judge the merits of RPC here > than I am. That leaves the question of the RPC transport. You want to > use QMP. I think that QMP is the more mature protocol than my RPC proof-of-concept and would improve interoperability. However, our open-source client stub supports both transport layers, so we're fine with both. > On the one hand, I'm tickled to see QAPI/QMP used for things it wasn't > designed for. Might be a language issue but I can't tell whether that's meant positively or negatively. > On the other hand, QMP has grown so big. Keeping it cohesive has become > a near-impossible mission. > > Hmm. > > We already provide another remote debugger interface: the GDB stub. > It's optional, i.e. the user has to create it, either with command line > option -gdb, or monitor command gdbserver. We had that option in our previous patch set: > $ qemu-system-arm -M virt -cpu cortex-a15 -mcd tcp::1235 If you'd like, we can of course add such an option again. The QMP choice made that redundant for me because I wasn't aware of a way to only provide a subset of QMP. > We already have two-and-a-half QMPs: qemu-system-FOO's QMP, > qemu-storage-daemon's QMP (subset of the previous, so it counts only > half), and qemu-ga's QMP-like protocol. > > What about providing the MCD interface as a separate QMP-like protocol? > It gets its own QAPI schema, just like for qemu-ga. Simplifies > compiling it out when not needed. > It gets its own socket, just like the GDB stub. Might reduce > interference between debugging and QMP. > > Thoughts? Alex, Philippe, care to chime in? Sound reasonable to me. Keeping in mind the size of generated QAPI code, an option to `./configure [...] --enable-mcd` is definitely advisable.