[PATCH v5 0/3] hw/uefi: add support for receiving the firmware log via monitor.

[PATCH v5 0/3] hw/uefi: add support for receiving the firmware log via monitor.

[Gerd Hoffmann]

v5:
 - address review comments.
 - split hmp command to separate patch.

Gerd Hoffmann (3):
  hw/uefi: add query-firmware-log monitor command
  hw/uefi: add 'info firmware-log' hmp monitor command.
  hw/uefi/ovmf-log: add maxsize parameter

 include/monitor/hmp.h      |   1 +
 hw/uefi/ovmf-log.c         | 286 +++++++++++++++++++++++++++++++++++++
 tests/qtest/qmp-cmd-test.c |   2 +
 hmp-commands-info.hx       |  13 ++
 hw/uefi/meson.build        |   2 +-
 qapi/machine.json          |  29 ++++
 6 files changed, 332 insertions(+), 1 deletion(-)
 create mode 100644 hw/uefi/ovmf-log.c

-- 
2.51.0

[PATCH v5 1/3] hw/uefi: add query-firmware-log monitor command

[Gerd Hoffmann]

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 the query-firmware-log both qmp monitor command
to read the firmware log.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/monitor/hmp.h      |   1 +
 hw/uefi/ovmf-log.c         | 233 +++++++++++++++++++++++++++++++++++++
 tests/qtest/qmp-cmd-test.c |   2 +
 hw/uefi/meson.build        |   2 +-
 qapi/machine.json          |  24 ++++
 5 files changed, 261 insertions(+), 1 deletion(-)
 create mode 100644 hw/uefi/ovmf-log.c

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 31bd812e5f41..0af272b52ac1 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -179,5 +179,6 @@ void hmp_boot_set(Monitor *mon, const QDict *qdict);
 void hmp_info_mtree(Monitor *mon, const QDict *qdict);
 void hmp_info_cryptodev(Monitor *mon, const QDict *qdict);
 void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
+void hmp_info_firmware_log(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/hw/uefi/ovmf-log.c b/hw/uefi/ovmf-log.c
new file mode 100644
index 000000000000..85dda15ab6ad
--- /dev/null
+++ b/hw/uefi/ovmf-log.c
@@ -0,0 +1,233 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * print ovmf debug log
+ *
+ * see OvmfPkg/Library/MemDebugLogLib/ in edk2
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qemu/target-info-qapi.h"
+#include "hw/boards.h"
+#include "hw/i386/x86.h"
+#include "hw/arm/virt.h"
+#include "system/dma.h"
+#include "monitor/hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/error.h"
+#include "qapi/type-helpers.h"
+#include "qapi/qapi-commands-machine.h"
+
+
+/* ----------------------------------------------------------------------- */
+/* copy from edk2                                                          */
+
+#define MEM_DEBUG_LOG_MAGIC1  0x3167646d666d766f  /* "ovmfmdg1" */
+#define MEM_DEBUG_LOG_MAGIC2  0x3267646d666d766f  /* "ovmfmdg2" */
+
+/*
+ * Mem Debug Log buffer header.
+ * The Log buffer is circular. Only the most
+ * recent messages are retained. Older messages
+ * will be discarded if the buffer overflows.
+ * The Debug Log starts just after the header.
+ */
+typedef struct {
+    /*
+     * Magic values
+     * These fields are used by tools to locate the buffer in
+     * memory. These MUST be the first two fields of the structure.
+     * Use a 128 bit Magic to vastly reduce the possibility of
+     * a collision with random data in memory.
+     */
+    uint64_t             Magic1;
+    uint64_t             Magic2;
+    /*
+     * Header Size
+     * This MUST be the third field of the structure
+     */
+    uint64_t             HeaderSize;
+    /*
+     * Debug log size (minus header)
+     */
+    uint64_t             DebugLogSize;
+    /*
+     * edk2 uses this for locking access.
+     */
+    uint64_t             MemDebugLogLock;
+    /*
+     * Debug log head offset
+     */
+    uint64_t             DebugLogHeadOffset;
+    /*
+     *  Debug log tail offset
+     */
+    uint64_t             DebugLogTailOffset;
+    /*
+     * Flag to indicate if the buffer wrapped and was thus truncated.
+     */
+    uint64_t             Truncated;
+    /*
+     * Firmware Build Version (PcdFirmwareVersionString)
+     */
+    char                 FirmwareVersion[128];
+} MEM_DEBUG_LOG_HDR;
+
+
+/* ----------------------------------------------------------------------- */
+/* qemu monitor command                                                    */
+
+typedef struct {
+    uint64_t             magic1;
+    uint64_t             magic2;
+} MemDebugLogMagic;
+
+/* find log buffer in guest memory by searching for the magic cookie */
+static dma_addr_t find_ovmf_log_range(dma_addr_t start, dma_addr_t end)
+{
+    static const MemDebugLogMagic magic = {
+        .magic1 = MEM_DEBUG_LOG_MAGIC1,
+        .magic2 = MEM_DEBUG_LOG_MAGIC2,
+    };
+    MemDebugLogMagic check;
+    dma_addr_t step = 4 * KiB;
+    dma_addr_t offset;
+
+    for (offset = start; offset < end; offset += step) {
+        if (dma_memory_read(&address_space_memory, offset,
+                            &check, sizeof(check),
+                            MEMTXATTRS_UNSPECIFIED)) {
+            /* dma error -> stop searching */
+            break;
+        }
+        if (memcmp(&magic, &check, sizeof(check)) == 0) {
+            return offset;
+        }
+    }
+    return (dma_addr_t)-1;
+}
+
+static dma_addr_t find_ovmf_log(void)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    dma_addr_t start, end, offset;
+
+    if (target_arch() == SYS_EMU_TARGET_X86_64 &&
+        object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
+        X86MachineState *x86ms = X86_MACHINE(ms);
+
+        /* early log buffer, static allocation in memfd, sec + early pei */
+        offset = find_ovmf_log_range(0x800000, 0x900000);
+        if (offset != -1) {
+            return offset;
+        }
+
+        /*
+         * normal log buffer, dynamically allocated close to end of low memory,
+         * late pei + dxe phase
+         */
+        end = x86ms->below_4g_mem_size;
+        start = end - MIN(end, 128 * MiB);
+        return find_ovmf_log_range(start, end);
+    }
+
+    if (target_arch() == SYS_EMU_TARGET_AARCH64 &&
+        object_dynamic_cast(OBJECT(ms), TYPE_VIRT_MACHINE)) {
+        VirtMachineState *vms = VIRT_MACHINE(ms);
+
+        /* edk2 ArmVirt firmware allocations are in the first 128 MB */
+        start = vms->memmap[VIRT_MEM].base;
+        end = start + 128 * MiB;
+        return find_ovmf_log_range(start, end);
+    }
+
+    return (dma_addr_t)-1;
+}
+
+static void handle_ovmf_log_range(GString *out,
+                                  dma_addr_t start,
+                                  dma_addr_t end,
+                                  Error **errp)
+{
+    if (start > end) {
+        return;
+    }
+
+    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)) {
+        error_setg(errp, "firmware log: buffer read error");
+        return;
+    }
+}
+
+FirmwareLog *qmp_query_firmware_log(Error **errp)
+{
+    MEM_DEBUG_LOG_HDR header;
+    dma_addr_t offset, base;
+    FirmwareLog *ret;
+    g_autoptr(GString) log = g_string_new("");
+
+    offset = find_ovmf_log();
+    if (offset == -1) {
+        error_setg(errp, "firmware log: not found");
+        return NULL;
+    }
+
+    if (dma_memory_read(&address_space_memory, offset,
+                        &header, sizeof(header),
+                        MEMTXATTRS_UNSPECIFIED)) {
+        error_setg(errp, "firmware log: header read error");
+        return NULL;
+    }
+
+    if (header.DebugLogSize > MiB) {
+        /* default size is 128k (32 pages), allow up to 1M */
+        error_setg(errp, "firmware log: log buffer is too big");
+        return NULL;
+    }
+
+    if (header.DebugLogHeadOffset > header.DebugLogSize ||
+        header.DebugLogTailOffset > header.DebugLogSize) {
+        error_setg(errp, "firmware log: invalid header");
+        return NULL;
+    }
+
+    base = offset + header.HeaderSize;
+    if (header.DebugLogHeadOffset > header.DebugLogTailOffset) {
+        /* wrap around */
+        handle_ovmf_log_range(log,
+                              base + header.DebugLogHeadOffset,
+                              base + header.DebugLogSize,
+                              errp);
+        if (*errp) {
+            return NULL;
+        }
+        handle_ovmf_log_range(log,
+                              base + 0,
+                              base + header.DebugLogTailOffset,
+                              errp);
+        if (*errp) {
+            return NULL;
+        }
+    } else {
+        handle_ovmf_log_range(log,
+                              base + header.DebugLogHeadOffset,
+                              base + header.DebugLogTailOffset,
+                              errp);
+        if (*errp) {
+            return NULL;
+        }
+    }
+
+    ret = g_new0(FirmwareLog, 1);
+    if (header.FirmwareVersion[0] != '\0') {
+        ret->version = g_strndup(header.FirmwareVersion,
+                                 sizeof(header.FirmwareVersion));
+    }
+    ret->log = g_base64_encode((const guchar *)log->str, log->len);
+    return ret;
+}
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index cf718761861d..279a8f5614e9 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -52,6 +52,8 @@ static int query_error_class(const char *cmd)
         /* Only valid with accel=tcg */
         { "x-query-jit", ERROR_CLASS_GENERIC_ERROR },
         { "xen-event-list", ERROR_CLASS_GENERIC_ERROR },
+        /* requires firmware with memory buffer logging support */
+        { "query-firmware-log", ERROR_CLASS_GENERIC_ERROR },
         { NULL, -1 }
     };
     int i;
diff --git a/hw/uefi/meson.build b/hw/uefi/meson.build
index 91eb95f89e6d..c8f38dfae247 100644
--- a/hw/uefi/meson.build
+++ b/hw/uefi/meson.build
@@ -1,4 +1,4 @@
-system_ss.add(files('hardware-info.c'))
+system_ss.add(files('hardware-info.c', 'ovmf-log.c'))
 
 uefi_vars_ss = ss.source_set()
 if (config_all_devices.has_key('CONFIG_UEFI_VARS'))
diff --git a/qapi/machine.json b/qapi/machine.json
index 038eab281c78..96133e5c71cf 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1839,6 +1839,30 @@
   'returns': 'HumanReadableText',
   'features': [ 'unstable' ]}
 
+##
+# @FirmwareLog:
+#
+# @version: Firmware version.
+#
+# @log: Firmware debug log, in base64 encoding.  First and last log
+#       line might be incomplete.
+#
+# Since: 10.2
+##
+{ 'struct': 'FirmwareLog',
+  'data': { '*version': 'str',
+            'log': 'str' } }
+
+##
+# @query-firmware-log:
+#
+# Find firmware memory log buffer in guest memory, return content.
+#
+# Since: 10.2
+##
+{ 'command': 'query-firmware-log',
+  'returns': 'FirmwareLog' }
+
 ##
 # @dump-skeys:
 #
-- 
2.51.0

[PATCH v5 2/3] hw/uefi: add 'info firmware-log' hmp monitor command.

[Gerd Hoffmann]

This adds the hmp variant of the query-firmware-log qmp command.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/uefi/ovmf-log.c   | 27 +++++++++++++++++++++++++++
 hmp-commands-info.hx | 13 +++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/hw/uefi/ovmf-log.c b/hw/uefi/ovmf-log.c
index 85dda15ab6ad..e281a980a101 100644
--- a/hw/uefi/ovmf-log.c
+++ b/hw/uefi/ovmf-log.c
@@ -231,3 +231,30 @@ FirmwareLog *qmp_query_firmware_log(Error **errp)
     ret->log = g_base64_encode((const guchar *)log->str, log->len);
     return ret;
 }
+
+void hmp_info_firmware_log(Monitor *mon, const QDict *qdict)
+{
+    g_autofree gchar *log_esc = NULL;
+    g_autofree guchar *log_out = NULL;
+    Error *err = NULL;
+    FirmwareLog *log;
+    gsize log_len;
+
+    log = qmp_query_firmware_log(&err);
+    if (err)  {
+        hmp_handle_error(mon, err);
+        return;
+    }
+
+    g_assert(log != NULL);
+    g_assert(log->log != NULL);
+
+    if (log->version) {
+        g_autofree gchar *esc = g_strescape(log->version, NULL);
+        monitor_printf(mon, "[ firmware version: %s ]\n", esc);
+    }
+
+    log_out = g_base64_decode(log->log, &log_len);
+    log_esc = g_strescape((gchar *)log_out, "\r\n");
+    monitor_printf(mon, "%s\n", log_esc);
+}
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index eaaa880c1b30..f0aef419923c 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -990,3 +990,16 @@ SRST
   ``info cryptodev``
     Show the crypto devices.
 ERST
+
+    {
+        .name       = "firmware-log",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show the firmware (ovmf) debug log",
+        .cmd        = hmp_info_firmware_log,
+    },
+
+SRST
+  ``info firmware-log``
+    Show the firmware (ovmf) debug log.
+ERST
-- 
2.51.0

[PATCH v5 3/3] hw/uefi/ovmf-log: add maxsize parameter

[Gerd Hoffmann]

Allow limiting the amount of log output sent.  Allow up to 1 MiB.
In case the guest log buffer is larger than 1 MiB limit the output
instead of throwing an error.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/uefi/ovmf-log.c   | 42 ++++++++++++++++++++++++++++++++++--------
 hmp-commands-info.hx |  4 ++--
 qapi/machine.json    |  5 +++++
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/hw/uefi/ovmf-log.c b/hw/uefi/ovmf-log.c
index e281a980a101..28788620e3f6 100644
--- a/hw/uefi/ovmf-log.c
+++ b/hw/uefi/ovmf-log.c
@@ -18,6 +18,7 @@
 #include "qapi/error.h"
 #include "qapi/type-helpers.h"
 #include "qapi/qapi-commands-machine.h"
+#include "qobject/qdict.h"
 
 
 /* ----------------------------------------------------------------------- */
@@ -164,7 +165,8 @@ static void handle_ovmf_log_range(GString *out,
     }
 }
 
-FirmwareLog *qmp_query_firmware_log(Error **errp)
+FirmwareLog *qmp_query_firmware_log(bool have_max_size, uint64_t max_size,
+                                    Error **errp)
 {
     MEM_DEBUG_LOG_HDR header;
     dma_addr_t offset, base;
@@ -184,18 +186,40 @@ FirmwareLog *qmp_query_firmware_log(Error **errp)
         return NULL;
     }
 
-    if (header.DebugLogSize > MiB) {
-        /* default size is 128k (32 pages), allow up to 1M */
-        error_setg(errp, "firmware log: log buffer is too big");
-        return NULL;
-    }
-
     if (header.DebugLogHeadOffset > header.DebugLogSize ||
         header.DebugLogTailOffset > header.DebugLogSize) {
         error_setg(errp, "firmware log: invalid header");
         return NULL;
     }
 
+    if (have_max_size) {
+        if (max_size > MiB) {
+            error_setg(errp, "firmware log: max-size larger than 1 MiB");
+            return NULL;
+        }
+    } else {
+        max_size = MiB;
+    }
+
+    /* adjust header.DebugLogHeadOffset so we return at most maxsize bytes */
+    if (header.DebugLogHeadOffset > header.DebugLogTailOffset) {
+        /* wrap around */
+        if (header.DebugLogTailOffset > max_size) {
+            header.DebugLogHeadOffset = header.DebugLogTailOffset - max_size;
+        } else {
+            uint64_t max_chunk = max_size - header.DebugLogTailOffset;
+            if (header.DebugLogSize > max_chunk &&
+                header.DebugLogHeadOffset < header.DebugLogSize - max_chunk) {
+                header.DebugLogHeadOffset = header.DebugLogSize - max_chunk;
+            }
+        }
+    } else {
+        if (header.DebugLogTailOffset > max_size &&
+            header.DebugLogHeadOffset < header.DebugLogTailOffset - max_size) {
+            header.DebugLogHeadOffset = header.DebugLogTailOffset - max_size;
+        }
+    }
+
     base = offset + header.HeaderSize;
     if (header.DebugLogHeadOffset > header.DebugLogTailOffset) {
         /* wrap around */
@@ -239,8 +263,10 @@ void hmp_info_firmware_log(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
     FirmwareLog *log;
     gsize log_len;
+    int64_t maxsize;
 
-    log = qmp_query_firmware_log(&err);
+    maxsize = qdict_get_try_int(qdict, "maxsize", -1);
+    log = qmp_query_firmware_log(maxsize != -1, (uint64_t)maxsize, &err);
     if (err)  {
         hmp_handle_error(mon, err);
         return;
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index f0aef419923c..f00d7081b40c 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -993,8 +993,8 @@ ERST
 
     {
         .name       = "firmware-log",
-        .args_type  = "",
-        .params     = "",
+        .args_type  = "maxsize:o?",
+        .params     = "[maxsize]",
         .help       = "show the firmware (ovmf) debug log",
         .cmd        = hmp_info_firmware_log,
     },
diff --git a/qapi/machine.json b/qapi/machine.json
index 96133e5c71cf..e4b15680c172 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1858,9 +1858,14 @@
 #
 # Find firmware memory log buffer in guest memory, return content.
 #
+# @max-size: limit the amount of log data returned.  Up to 1 MiB if
+#            log data is allowed.  In case the amout of log data is
+#            larger than max-size the tail of the log is returned.
+#
 # Since: 10.2
 ##
 { 'command': 'query-firmware-log',
+  'data': { '*max-size': 'size' },
   'returns': 'FirmwareLog' }
 
 ##
-- 
2.51.0

Re: [PATCH v5 1/3] hw/uefi: add query-firmware-log monitor command

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> 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 the query-firmware-log both qmp monitor command
> to read the firmware log.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/monitor/hmp.h      |   1 +
>  hw/uefi/ovmf-log.c         | 233 +++++++++++++++++++++++++++++++++++++
>  tests/qtest/qmp-cmd-test.c |   2 +
>  hw/uefi/meson.build        |   2 +-
>  qapi/machine.json          |  24 ++++
>  5 files changed, 261 insertions(+), 1 deletion(-)
>  create mode 100644 hw/uefi/ovmf-log.c
>
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index 31bd812e5f41..0af272b52ac1 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -179,5 +179,6 @@ void hmp_boot_set(Monitor *mon, const QDict *qdict);
>  void hmp_info_mtree(Monitor *mon, const QDict *qdict);
>  void hmp_info_cryptodev(Monitor *mon, const QDict *qdict);
>  void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
> +void hmp_info_firmware_log(Monitor *mon, const QDict *qdict);
>  
>  #endif

This hunk needs to go into the next patch.

> diff --git a/hw/uefi/ovmf-log.c b/hw/uefi/ovmf-log.c
> new file mode 100644
> index 000000000000..85dda15ab6ad
> --- /dev/null
> +++ b/hw/uefi/ovmf-log.c
> @@ -0,0 +1,233 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * print ovmf debug log
> + *
> + * see OvmfPkg/Library/MemDebugLogLib/ in edk2
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "qemu/target-info-qapi.h"
> +#include "hw/boards.h"
> +#include "hw/i386/x86.h"
> +#include "hw/arm/virt.h"
> +#include "system/dma.h"
> +#include "monitor/hmp.h"
> +#include "monitor/monitor.h"
> +#include "qapi/error.h"
> +#include "qapi/type-helpers.h"
> +#include "qapi/qapi-commands-machine.h"
> +
> +
> +/* ----------------------------------------------------------------------- */
> +/* copy from edk2                                                          */
> +
> +#define MEM_DEBUG_LOG_MAGIC1  0x3167646d666d766f  /* "ovmfmdg1" */
> +#define MEM_DEBUG_LOG_MAGIC2  0x3267646d666d766f  /* "ovmfmdg2" */
> +
> +/*
> + * Mem Debug Log buffer header.
> + * The Log buffer is circular. Only the most
> + * recent messages are retained. Older messages
> + * will be discarded if the buffer overflows.
> + * The Debug Log starts just after the header.
> + */
> +typedef struct {
> +    /*
> +     * Magic values
> +     * These fields are used by tools to locate the buffer in
> +     * memory. These MUST be the first two fields of the structure.
> +     * Use a 128 bit Magic to vastly reduce the possibility of
> +     * a collision with random data in memory.
> +     */
> +    uint64_t             Magic1;
> +    uint64_t             Magic2;
> +    /*
> +     * Header Size
> +     * This MUST be the third field of the structure
> +     */
> +    uint64_t             HeaderSize;
> +    /*
> +     * Debug log size (minus header)
> +     */
> +    uint64_t             DebugLogSize;
> +    /*
> +     * edk2 uses this for locking access.
> +     */
> +    uint64_t             MemDebugLogLock;
> +    /*
> +     * Debug log head offset
> +     */
> +    uint64_t             DebugLogHeadOffset;
> +    /*
> +     *  Debug log tail offset
> +     */
> +    uint64_t             DebugLogTailOffset;
> +    /*
> +     * Flag to indicate if the buffer wrapped and was thus truncated.
> +     */
> +    uint64_t             Truncated;
> +    /*
> +     * Firmware Build Version (PcdFirmwareVersionString)
> +     */
> +    char                 FirmwareVersion[128];
> +} MEM_DEBUG_LOG_HDR;
> +
> +
> +/* ----------------------------------------------------------------------- */
> +/* qemu monitor command                                                    */
> +
> +typedef struct {
> +    uint64_t             magic1;
> +    uint64_t             magic2;
> +} MemDebugLogMagic;
> +
> +/* find log buffer in guest memory by searching for the magic cookie */
> +static dma_addr_t find_ovmf_log_range(dma_addr_t start, dma_addr_t end)
> +{
> +    static const MemDebugLogMagic magic = {
> +        .magic1 = MEM_DEBUG_LOG_MAGIC1,
> +        .magic2 = MEM_DEBUG_LOG_MAGIC2,
> +    };
> +    MemDebugLogMagic check;
> +    dma_addr_t step = 4 * KiB;
> +    dma_addr_t offset;
> +
> +    for (offset = start; offset < end; offset += step) {
> +        if (dma_memory_read(&address_space_memory, offset,
> +                            &check, sizeof(check),
> +                            MEMTXATTRS_UNSPECIFIED)) {
> +            /* dma error -> stop searching */
> +            break;
> +        }
> +        if (memcmp(&magic, &check, sizeof(check)) == 0) {
> +            return offset;
> +        }
> +    }
> +    return (dma_addr_t)-1;
> +}
> +
> +static dma_addr_t find_ovmf_log(void)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    dma_addr_t start, end, offset;
> +
> +    if (target_arch() == SYS_EMU_TARGET_X86_64 &&
> +        object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
> +        X86MachineState *x86ms = X86_MACHINE(ms);
> +
> +        /* early log buffer, static allocation in memfd, sec + early pei */
> +        offset = find_ovmf_log_range(0x800000, 0x900000);
> +        if (offset != -1) {
> +            return offset;
> +        }
> +
> +        /*
> +         * normal log buffer, dynamically allocated close to end of low memory,
> +         * late pei + dxe phase
> +         */
> +        end = x86ms->below_4g_mem_size;
> +        start = end - MIN(end, 128 * MiB);
> +        return find_ovmf_log_range(start, end);
> +    }
> +
> +    if (target_arch() == SYS_EMU_TARGET_AARCH64 &&
> +        object_dynamic_cast(OBJECT(ms), TYPE_VIRT_MACHINE)) {
> +        VirtMachineState *vms = VIRT_MACHINE(ms);
> +
> +        /* edk2 ArmVirt firmware allocations are in the first 128 MB */
> +        start = vms->memmap[VIRT_MEM].base;
> +        end = start + 128 * MiB;
> +        return find_ovmf_log_range(start, end);
> +    }
> +
> +    return (dma_addr_t)-1;
> +}
> +
> +static void handle_ovmf_log_range(GString *out,
> +                                  dma_addr_t start,
> +                                  dma_addr_t end,
> +                                  Error **errp)
> +{
> +    if (start > end) {
> +        return;
> +    }
> +
> +    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)) {
> +        error_setg(errp, "firmware log: buffer read error");
> +        return;
> +    }
> +}
> +
> +FirmwareLog *qmp_query_firmware_log(Error **errp)
> +{
> +    MEM_DEBUG_LOG_HDR header;
> +    dma_addr_t offset, base;
> +    FirmwareLog *ret;
> +    g_autoptr(GString) log = g_string_new("");
> +
> +    offset = find_ovmf_log();
> +    if (offset == -1) {
> +        error_setg(errp, "firmware log: not found");
> +        return NULL;
> +    }
> +
> +    if (dma_memory_read(&address_space_memory, offset,
> +                        &header, sizeof(header),
> +                        MEMTXATTRS_UNSPECIFIED)) {
> +        error_setg(errp, "firmware log: header read error");
> +        return NULL;
> +    }
> +
> +    if (header.DebugLogSize > MiB) {
> +        /* default size is 128k (32 pages), allow up to 1M */
> +        error_setg(errp, "firmware log: log buffer is too big");
> +        return NULL;
> +    }
> +
> +    if (header.DebugLogHeadOffset > header.DebugLogSize ||
> +        header.DebugLogTailOffset > header.DebugLogSize) {
> +        error_setg(errp, "firmware log: invalid header");
> +        return NULL;
> +    }
> +
> +    base = offset + header.HeaderSize;
> +    if (header.DebugLogHeadOffset > header.DebugLogTailOffset) {
> +        /* wrap around */
> +        handle_ovmf_log_range(log,
> +                              base + header.DebugLogHeadOffset,
> +                              base + header.DebugLogSize,
> +                              errp);
> +        if (*errp) {
> +            return NULL;
> +        }
> +        handle_ovmf_log_range(log,
> +                              base + 0,
> +                              base + header.DebugLogTailOffset,
> +                              errp);
> +        if (*errp) {
> +            return NULL;
> +        }
> +    } else {
> +        handle_ovmf_log_range(log,
> +                              base + header.DebugLogHeadOffset,
> +                              base + header.DebugLogTailOffset,
> +                              errp);
> +        if (*errp) {
> +            return NULL;
> +        }
> +    }
> +
> +    ret = g_new0(FirmwareLog, 1);
> +    if (header.FirmwareVersion[0] != '\0') {
> +        ret->version = g_strndup(header.FirmwareVersion,
> +                                 sizeof(header.FirmwareVersion));
> +    }

In this revision, @version is present when the Firmware supports it, and
its not the empty string.

The previous revision had

       ret->version = g_strndup(header.FirmwareVersion,
                                sizeof(header.FirmwareVersion));

Present when the Firmware supports it.

I'm not sure we care for the difference.

Observation, not a demand for antthing.

> +    ret->log = g_base64_encode((const guchar *)log->str, log->len);
> +    return ret;
> +}
> diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
> index cf718761861d..279a8f5614e9 100644
> --- a/tests/qtest/qmp-cmd-test.c
> +++ b/tests/qtest/qmp-cmd-test.c
> @@ -52,6 +52,8 @@ static int query_error_class(const char *cmd)
>          /* Only valid with accel=tcg */
>          { "x-query-jit", ERROR_CLASS_GENERIC_ERROR },
>          { "xen-event-list", ERROR_CLASS_GENERIC_ERROR },
> +        /* requires firmware with memory buffer logging support */
> +        { "query-firmware-log", ERROR_CLASS_GENERIC_ERROR },
>          { NULL, -1 }
>      };
>      int i;
> diff --git a/hw/uefi/meson.build b/hw/uefi/meson.build
> index 91eb95f89e6d..c8f38dfae247 100644
> --- a/hw/uefi/meson.build
> +++ b/hw/uefi/meson.build
> @@ -1,4 +1,4 @@
> -system_ss.add(files('hardware-info.c'))
> +system_ss.add(files('hardware-info.c', 'ovmf-log.c'))
>  
>  uefi_vars_ss = ss.source_set()
>  if (config_all_devices.has_key('CONFIG_UEFI_VARS'))
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 038eab281c78..96133e5c71cf 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1839,6 +1839,30 @@
>    'returns': 'HumanReadableText',
>    'features': [ 'unstable' ]}
>  
> +##
> +# @FirmwareLog:
> +#
> +# @version: Firmware version.
> +#
> +# @log: Firmware debug log, in base64 encoding.  First and last log
> +#       line might be incomplete.
> +#
> +# Since: 10.2
> +##
> +{ 'struct': 'FirmwareLog',
> +  'data': { '*version': 'str',
> +            'log': 'str' } }
> +
> +##
> +# @query-firmware-log:
> +#
> +# Find firmware memory log buffer in guest memory, return content.
> +#
> +# Since: 10.2
> +##
> +{ 'command': 'query-firmware-log',
> +  'returns': 'FirmwareLog' }
> +
>  ##
>  # @dump-skeys:
>  #

With the first hunk moved
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v5 2/3] hw/uefi: add 'info firmware-log' hmp monitor command.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> This adds the hmp variant of the query-firmware-log qmp command.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/uefi/ovmf-log.c   | 27 +++++++++++++++++++++++++++
>  hmp-commands-info.hx | 13 +++++++++++++
>  2 files changed, 40 insertions(+)
>
> diff --git a/hw/uefi/ovmf-log.c b/hw/uefi/ovmf-log.c
> index 85dda15ab6ad..e281a980a101 100644
> --- a/hw/uefi/ovmf-log.c
> +++ b/hw/uefi/ovmf-log.c
> @@ -231,3 +231,30 @@ FirmwareLog *qmp_query_firmware_log(Error **errp)
>      ret->log = g_base64_encode((const guchar *)log->str, log->len);
>      return ret;
>  }
> +
> +void hmp_info_firmware_log(Monitor *mon, const QDict *qdict)
> +{
> +    g_autofree gchar *log_esc = NULL;
> +    g_autofree guchar *log_out = NULL;
> +    Error *err = NULL;
> +    FirmwareLog *log;
> +    gsize log_len;
> +
> +    log = qmp_query_firmware_log(&err);
> +    if (err)  {
> +        hmp_handle_error(mon, err);
> +        return;
> +    }
> +
> +    g_assert(log != NULL);
> +    g_assert(log->log != NULL);

QAPI ensures this, so I wouldn't bother myself.  Up to you.

> +
> +    if (log->version) {
> +        g_autofree gchar *esc = g_strescape(log->version, NULL);
> +        monitor_printf(mon, "[ firmware version: %s ]\n", esc);
> +    }
> +
> +    log_out = g_base64_decode(log->log, &log_len);
> +    log_esc = g_strescape((gchar *)log_out, "\r\n");
> +    monitor_printf(mon, "%s\n", log_esc);
> +}
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index eaaa880c1b30..f0aef419923c 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -990,3 +990,16 @@ SRST
>    ``info cryptodev``
>      Show the crypto devices.
>  ERST
> +
> +    {
> +        .name       = "firmware-log",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the firmware (ovmf) debug log",
> +        .cmd        = hmp_info_firmware_log,
> +    },
> +
> +SRST
> +  ``info firmware-log``
> +    Show the firmware (ovmf) debug log.
> +ERST

With the update of monitor/hmp.h moved here from PATCH 1
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v5 1/3] hw/uefi: add query-firmware-log monitor command

[Markus Armbruster]

Second thoughts on error handling...

Gerd Hoffmann <kraxel@redhat.com> 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 the query-firmware-log both qmp monitor command
> to read the firmware log.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/monitor/hmp.h      |   1 +
>  hw/uefi/ovmf-log.c         | 233 +++++++++++++++++++++++++++++++++++++
>  tests/qtest/qmp-cmd-test.c |   2 +
>  hw/uefi/meson.build        |   2 +-
>  qapi/machine.json          |  24 ++++
>  5 files changed, 261 insertions(+), 1 deletion(-)
>  create mode 100644 hw/uefi/ovmf-log.c
>
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index 31bd812e5f41..0af272b52ac1 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -179,5 +179,6 @@ void hmp_boot_set(Monitor *mon, const QDict *qdict);
>  void hmp_info_mtree(Monitor *mon, const QDict *qdict);
>  void hmp_info_cryptodev(Monitor *mon, const QDict *qdict);
>  void hmp_dumpdtb(Monitor *mon, const QDict *qdict);
> +void hmp_info_firmware_log(Monitor *mon, const QDict *qdict);
>  
>  #endif
> diff --git a/hw/uefi/ovmf-log.c b/hw/uefi/ovmf-log.c
> new file mode 100644
> index 000000000000..85dda15ab6ad
> --- /dev/null
> +++ b/hw/uefi/ovmf-log.c
> @@ -0,0 +1,233 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * print ovmf debug log
> + *
> + * see OvmfPkg/Library/MemDebugLogLib/ in edk2
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "qemu/target-info-qapi.h"
> +#include "hw/boards.h"
> +#include "hw/i386/x86.h"
> +#include "hw/arm/virt.h"
> +#include "system/dma.h"
> +#include "monitor/hmp.h"
> +#include "monitor/monitor.h"
> +#include "qapi/error.h"
> +#include "qapi/type-helpers.h"
> +#include "qapi/qapi-commands-machine.h"
> +
> +
> +/* ----------------------------------------------------------------------- */
> +/* copy from edk2                                                          */
> +
> +#define MEM_DEBUG_LOG_MAGIC1  0x3167646d666d766f  /* "ovmfmdg1" */
> +#define MEM_DEBUG_LOG_MAGIC2  0x3267646d666d766f  /* "ovmfmdg2" */
> +
> +/*
> + * Mem Debug Log buffer header.
> + * The Log buffer is circular. Only the most
> + * recent messages are retained. Older messages
> + * will be discarded if the buffer overflows.
> + * The Debug Log starts just after the header.
> + */
> +typedef struct {
> +    /*
> +     * Magic values
> +     * These fields are used by tools to locate the buffer in
> +     * memory. These MUST be the first two fields of the structure.
> +     * Use a 128 bit Magic to vastly reduce the possibility of
> +     * a collision with random data in memory.
> +     */
> +    uint64_t             Magic1;
> +    uint64_t             Magic2;
> +    /*
> +     * Header Size
> +     * This MUST be the third field of the structure
> +     */
> +    uint64_t             HeaderSize;
> +    /*
> +     * Debug log size (minus header)
> +     */
> +    uint64_t             DebugLogSize;
> +    /*
> +     * edk2 uses this for locking access.
> +     */
> +    uint64_t             MemDebugLogLock;
> +    /*
> +     * Debug log head offset
> +     */
> +    uint64_t             DebugLogHeadOffset;
> +    /*
> +     *  Debug log tail offset
> +     */
> +    uint64_t             DebugLogTailOffset;
> +    /*
> +     * Flag to indicate if the buffer wrapped and was thus truncated.
> +     */
> +    uint64_t             Truncated;
> +    /*
> +     * Firmware Build Version (PcdFirmwareVersionString)
> +     */
> +    char                 FirmwareVersion[128];
> +} MEM_DEBUG_LOG_HDR;
> +
> +
> +/* ----------------------------------------------------------------------- */
> +/* qemu monitor command                                                    */
> +
> +typedef struct {
> +    uint64_t             magic1;
> +    uint64_t             magic2;
> +} MemDebugLogMagic;
> +
> +/* find log buffer in guest memory by searching for the magic cookie */
> +static dma_addr_t find_ovmf_log_range(dma_addr_t start, dma_addr_t end)
> +{
> +    static const MemDebugLogMagic magic = {
> +        .magic1 = MEM_DEBUG_LOG_MAGIC1,
> +        .magic2 = MEM_DEBUG_LOG_MAGIC2,
> +    };
> +    MemDebugLogMagic check;
> +    dma_addr_t step = 4 * KiB;
> +    dma_addr_t offset;
> +
> +    for (offset = start; offset < end; offset += step) {
> +        if (dma_memory_read(&address_space_memory, offset,
> +                            &check, sizeof(check),
> +                            MEMTXATTRS_UNSPECIFIED)) {
> +            /* dma error -> stop searching */
> +            break;
> +        }
> +        if (memcmp(&magic, &check, sizeof(check)) == 0) {
> +            return offset;
> +        }
> +    }
> +    return (dma_addr_t)-1;
> +}

This searches a well-known address range for a page starting with 128
magic bits.

It uses dma_memory_read(), so the address range is in the DMA
controller's address space.  I assume that's okay.

dma_memory_read() fails basically when nothing is mapped in the DMA
controller's address space there.

If the function succeeds, we should have an entire page, as long as
nothing else messes with the mapping.

> +
> +static dma_addr_t find_ovmf_log(void)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    dma_addr_t start, end, offset;
> +
> +    if (target_arch() == SYS_EMU_TARGET_X86_64 &&
> +        object_dynamic_cast(OBJECT(ms), TYPE_X86_MACHINE)) {
> +        X86MachineState *x86ms = X86_MACHINE(ms);
> +
> +        /* early log buffer, static allocation in memfd, sec + early pei */
> +        offset = find_ovmf_log_range(0x800000, 0x900000);
> +        if (offset != -1) {
> +            return offset;
> +        }
> +
> +        /*
> +         * normal log buffer, dynamically allocated close to end of low memory,
> +         * late pei + dxe phase
> +         */
> +        end = x86ms->below_4g_mem_size;
> +        start = end - MIN(end, 128 * MiB);
> +        return find_ovmf_log_range(start, end);
> +    }
> +
> +    if (target_arch() == SYS_EMU_TARGET_AARCH64 &&
> +        object_dynamic_cast(OBJECT(ms), TYPE_VIRT_MACHINE)) {
> +        VirtMachineState *vms = VIRT_MACHINE(ms);
> +
> +        /* edk2 ArmVirt firmware allocations are in the first 128 MB */
> +        start = vms->memmap[VIRT_MEM].base;
> +        end = start + 128 * MiB;
> +        return find_ovmf_log_range(start, end);
> +    }
> +
> +    return (dma_addr_t)-1;
> +}

Again, if this succeeds, we should have an entire page.

> +
> +static void handle_ovmf_log_range(GString *out,
> +                                  dma_addr_t start,
> +                                  dma_addr_t end,
> +                                  Error **errp)
> +{
> +    if (start > end) {
> +        return;
> +    }
> +
> +    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)) {
> +        error_setg(errp, "firmware log: buffer read error");

I'm no friend of the error message format "what we're dealing with: what
went wrong".  What we're dealing with is usually obvious.  Suggest
something like "can't read firmware log buffer contents".

> +        return;
> +    }
> +}
> +
> +FirmwareLog *qmp_query_firmware_log(Error **errp)
> +{
> +    MEM_DEBUG_LOG_HDR header;
> +    dma_addr_t offset, base;
> +    FirmwareLog *ret;
> +    g_autoptr(GString) log = g_string_new("");
> +
> +    offset = find_ovmf_log();
> +    if (offset == -1) {
> +        error_setg(errp, "firmware log: not found");

"firmware log buffer not found"

> +        return NULL;
> +    }
> +
> +    if (dma_memory_read(&address_space_memory, offset,
> +                        &header, sizeof(header),
> +                        MEMTXATTRS_UNSPECIFIED)) {
> +        error_setg(errp, "firmware log: header read error");

Can this actually happen?  We should have an entire page...

"can't read firmware log buffer header".

> +        return NULL;
> +    }
> +
> +    if (header.DebugLogSize > MiB) {
> +        /* default size is 128k (32 pages), allow up to 1M */
> +        error_setg(errp, "firmware log: log buffer is too big");

"firmware log buffer exceeds 1MiB"

This error goes away in PATCH 3.

> +        return NULL;
> +    }
> +
> +    if (header.DebugLogHeadOffset > header.DebugLogSize ||
> +        header.DebugLogTailOffset > header.DebugLogSize) {
> +        error_setg(errp, "firmware log: invalid header");

"firmware log buffer header is invalid"

Could also treat it the same as bad magic, i.e. check it together with
!find_ovmf_log().

> +        return NULL;
> +    }
> +
> +    base = offset + header.HeaderSize;
> +    if (header.DebugLogHeadOffset > header.DebugLogTailOffset) {
> +        /* wrap around */
> +        handle_ovmf_log_range(log,
> +                              base + header.DebugLogHeadOffset,
> +                              base + header.DebugLogSize,
> +                              errp);
> +        if (*errp) {
> +            return NULL;
> +        }
> +        handle_ovmf_log_range(log,
> +                              base + 0,
> +                              base + header.DebugLogTailOffset,
> +                              errp);
> +        if (*errp) {
> +            return NULL;
> +        }
> +    } else {
> +        handle_ovmf_log_range(log,
> +                              base + header.DebugLogHeadOffset,
> +                              base + header.DebugLogTailOffset,
> +                              errp);
> +        if (*errp) {
> +            return NULL;
> +        }
> +    }
> +
> +    ret = g_new0(FirmwareLog, 1);
> +    if (header.FirmwareVersion[0] != '\0') {
> +        ret->version = g_strndup(header.FirmwareVersion,
> +                                 sizeof(header.FirmwareVersion));
> +    }
> +    ret->log = g_base64_encode((const guchar *)log->str, log->len);
> +    return ret;
> +}

[...]

Re: [PATCH v5 3/3] hw/uefi/ovmf-log: add maxsize parameter

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Allow limiting the amount of log output sent.  Allow up to 1 MiB.
> In case the guest log buffer is larger than 1 MiB limit the output
> instead of throwing an error.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/uefi/ovmf-log.c   | 42 ++++++++++++++++++++++++++++++++++--------
>  hmp-commands-info.hx |  4 ++--
>  qapi/machine.json    |  5 +++++
>  3 files changed, 41 insertions(+), 10 deletions(-)
>
> diff --git a/hw/uefi/ovmf-log.c b/hw/uefi/ovmf-log.c
> index e281a980a101..28788620e3f6 100644
> --- a/hw/uefi/ovmf-log.c
> +++ b/hw/uefi/ovmf-log.c
> @@ -18,6 +18,7 @@
>  #include "qapi/error.h"
>  #include "qapi/type-helpers.h"
>  #include "qapi/qapi-commands-machine.h"
> +#include "qobject/qdict.h"
>  
>  
>  /* ----------------------------------------------------------------------- */
> @@ -164,7 +165,8 @@ static void handle_ovmf_log_range(GString *out,
>      }
>  }
>  
> -FirmwareLog *qmp_query_firmware_log(Error **errp)
> +FirmwareLog *qmp_query_firmware_log(bool have_max_size, uint64_t max_size,
> +                                    Error **errp)
>  {
>      MEM_DEBUG_LOG_HDR header;
>      dma_addr_t offset, base;
> @@ -184,18 +186,40 @@ FirmwareLog *qmp_query_firmware_log(Error **errp)
>          return NULL;
>      }
>  
> -    if (header.DebugLogSize > MiB) {
> -        /* default size is 128k (32 pages), allow up to 1M */
> -        error_setg(errp, "firmware log: log buffer is too big");
> -        return NULL;
> -    }
> -
>      if (header.DebugLogHeadOffset > header.DebugLogSize ||
>          header.DebugLogTailOffset > header.DebugLogSize) {
>          error_setg(errp, "firmware log: invalid header");
>          return NULL;
>      }
>  
> +    if (have_max_size) {
> +        if (max_size > MiB) {
> +            error_setg(errp, "firmware log: max-size larger than 1 MiB");

"parameter 'max-size' exceeds 1MiB" or similar.

> +            return NULL;
> +        }
> +    } else {
> +        max_size = MiB;
> +    }
> +
> +    /* adjust header.DebugLogHeadOffset so we return at most maxsize bytes */
> +    if (header.DebugLogHeadOffset > header.DebugLogTailOffset) {
> +        /* wrap around */
> +        if (header.DebugLogTailOffset > max_size) {
> +            header.DebugLogHeadOffset = header.DebugLogTailOffset - max_size;
> +        } else {
> +            uint64_t max_chunk = max_size - header.DebugLogTailOffset;
> +            if (header.DebugLogSize > max_chunk &&
> +                header.DebugLogHeadOffset < header.DebugLogSize - max_chunk) {
> +                header.DebugLogHeadOffset = header.DebugLogSize - max_chunk;
> +            }
> +        }
> +    } else {
> +        if (header.DebugLogTailOffset > max_size &&
> +            header.DebugLogHeadOffset < header.DebugLogTailOffset - max_size) {
> +            header.DebugLogHeadOffset = header.DebugLogTailOffset - max_size;
> +        }
> +    }

Didn't review this part; my brain's fried for today :)

> +
>      base = offset + header.HeaderSize;
>      if (header.DebugLogHeadOffset > header.DebugLogTailOffset) {
>          /* wrap around */
> @@ -239,8 +263,10 @@ void hmp_info_firmware_log(Monitor *mon, const QDict *qdict)
>      Error *err = NULL;
>      FirmwareLog *log;
>      gsize log_len;
> +    int64_t maxsize;
>  
> -    log = qmp_query_firmware_log(&err);
> +    maxsize = qdict_get_try_int(qdict, "maxsize", -1);
> +    log = qmp_query_firmware_log(maxsize != -1, (uint64_t)maxsize, &err);
>      if (err)  {
>          hmp_handle_error(mon, err);
>          return;
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index f0aef419923c..f00d7081b40c 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -993,8 +993,8 @@ ERST
>  
>      {
>          .name       = "firmware-log",
> -        .args_type  = "",
> -        .params     = "",
> +        .args_type  = "maxsize:o?",
> +        .params     = "[maxsize]",
>          .help       = "show the firmware (ovmf) debug log",
>          .cmd        = hmp_info_firmware_log,
>      },

Might want to spell it max-size for consistency with QMP.  Up to you.

> diff --git a/qapi/machine.json b/qapi/machine.json
> index 96133e5c71cf..e4b15680c172 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1858,9 +1858,14 @@
>  #
>  # Find firmware memory log buffer in guest memory, return content.
>  #
> +# @max-size: limit the amount of log data returned.  Up to 1 MiB if
> +#            log data is allowed.  In case the amout of log data is

What are you trying to convey by "if log data is allowed"?

s/amout/amount/

> +#            larger than max-size the tail of the log is returned.

Please use @max-size here for nicer rendering.

> +#
>  # Since: 10.2
>  ##
>  { 'command': 'query-firmware-log',
> +  'data': { '*max-size': 'size' },
>    'returns': 'FirmwareLog' }
>  
>  ##