Re: [PATCH v3] hw/uefi: add "info firmware-log" + "query-firmware-log" monitor commands

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Oct 10, 2025 at 09:10:08AM +0200, Gerd Hoffmann wrote:
> 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.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/monitor/hmp.h      |   1 +
>  hw/uefi/ovmf-log.c         | 265 +++++++++++++++++++++++++++++++++++++
>  tests/qtest/qmp-cmd-test.c |   2 +
>  hmp-commands-info.hx       |  14 ++
>  hw/uefi/meson.build        |   2 +-
>  qapi/machine.json          |  23 ++++
>  6 files changed, 306 insertions(+), 1 deletion(-)
>  create mode 100644 hw/uefi/ovmf-log.c

> diff --git a/hw/uefi/ovmf-log.c b/hw/uefi/ovmf-log.c
> new file mode 100644
> index 000000000000..89e27d916531
> --- /dev/null
> +++ b/hw/uefi/ovmf-log.c
> @@ -0,0 +1,265 @@

> +static void handle_ovmf_log_range(GString *out,
> +                                  dma_addr_t start,
> +                                  dma_addr_t end,
> +                                  Error **errp)
> +{
> +    g_autofree char *buf = NULL;
> +
> +    if (start > end) {
> +        return;
> +    }
> +
> +    buf = g_malloc(end - start + 1);
> +    if (dma_memory_read(&address_space_memory, start,
> +                        buf, end - start,
> +                        MEMTXATTRS_UNSPECIFIED)) {
> +        error_setg(errp, "firmware log: buffer read error");
> +        return;
> +    }
> +
> +    buf[end - start] = 0;
> +    g_string_append_printf(out, "%s", buf);

How about eliminating the intermediate buffer alloocation / printf by
reading straight into the GString buf ? Something like

   size_t len = end - start;
   g_string_set_size(out, out->len + len);
   if (dma_memory_read(&address_space_memory, start,
                       out->str + (out->len - len),
	               len, MEMTXATTRS_UNSPECIFIED)) {
       ...
   }

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|