On Fri, Oct 10, 2025 at 01:41:35PM +0200, Markus Armbruster wrote:
> Gerd Hoffmann <[email protected]> writes:
>
> > Starting with the edk2-stable202508 tag OVMF (and ArmVirt too) have
> > optional support for logging to a memory buffer. There is guest side
> > support -- for example in linux kernels v6.17+ -- to read that buffer.
> > But that might not helpful if your guest stops booting early enough that
> > guest tooling can not be used yet. So host side support to read that
> > log buffer is a useful thing to have.
> >
> > This patch implements both qmp and hmp monitor commands to read the
> > firmware log.
>
> So this is just for EDK2, at least for now.
Yes.
> > + char FirmwareVersion[128];
> > +} MEM_DEBUG_LOG_HDR;
>
> I understand this is a (close to) literal copy from EDK2, and adjusting
> it to QEMU style would be a bad idea.
>
> > +
> > +
> > +/* -----------------------------------------------------------------------
> > */
> > +/* qemu monitor command
> > */
> > +
> > +typedef struct {
> > + uint64_t Magic1;
> > + uint64_t Magic2;
> > +} MEM_DEBUG_LOG_MAGIC;
>
> Unusual capitalization for a typedef name. Why? To emphasize the
> relation to MEM_DEBUG_LOG_HDR?
Yes.
> > + if (header.DebugLogSize > MiB) {
> > + /* default size is 128k (32 pages), allow up to 1M */
> > + error_setg(errp, "firmware log: log buffer is too big");
>
> [*] We limit the buffer to 1MiB. No objection to the size.
>
> What do you mean by "default" in "default size"? Is the size
> configurable in EDK2?
Yes, there is an option for that.
> Should we try to cope more gracefully with oversized log buffers? It's
> a ring buffer. What about silently reading the latest 1MiB then?
> Behaves just as if the ring buffer was 1MiB.
See below.
> > +# @log: Firmware debug log, in base64 encoding.
>
> Can this have a partial line at the beginning and/or the end?
Yes.
> > +#
> > +# Since: 10.2
> > +##
> > +{ 'struct': 'FirmwareLog',
> > + 'data': { '*version': 'str',
> > + '*log': 'str' } }
>
> These aren't actually optional with the current code. See [**] above.
> I guess you make them optional just in case some other firmware can
> provide only one of them.
We could also make both mandatory. There is always the option to return
an empty string ...
> > +##
> > +# @query-firmware-log:
> > +#
> > +# Find firmware memory log buffer in guest memory, return content.
>
> Should we mention this is implemented only for EDK2 at this time?
>
> Have you considered an optional size argument to retrieve the tail of
> the log?
I'll have a look. If we implement the 1MB cap suggested above we would
get that (almost) for free.
take care,
Gerd
