Re: [PATCH v3 6/6] igvm: Fill MADT IGVM parameter field

[Luigi Leonardi <leonardi@redhat.com>]

On Fri, Jan 09, 2026 at 03:34:13PM +0100, Oliver Steffen wrote:
>Use the new acpi_build_madt_standalone() function to fill the MADT
>parameter field.
>
>The IGVM parameter can be consumed by Coconut SVSM [1], instead of
>relying on the fw_cfg interface, which has caused problems before due to
>unexpected access [2,3]. Using IGVM parameters is the default way for
>Coconut SVSM; switching over would allow removing specialized code paths
>for QEMU in Coconut.
>
>In any case OVMF, which runs after SVSM has already been initialized,
>will continue reading all ACPI tables via fw_cfg and provide fixed up
>ACPI data to the OS as before.
>
>Generating the MADT twice (during ACPI table building and IGVM processing)
>seems acceptable, since there is no infrastructure to obtain the MADT
>out of the ACPI table memory area.
>
>[1] https://github.com/coconut-svsm/svsm/pull/858
>[2] https://gitlab.com/qemu-project/qemu/-/issues/2882
>[3] https://github.com/coconut-svsm/svsm/issues/646
>
>Signed-off-by: Oliver Steffen <osteffen@redhat.com>
>
>SQUASH: Rename madt parameter handler

Development leftover?

>---
> backends/igvm.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 7390dee734..90ea2c22fd 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -15,9 +15,11 @@
> #include "qapi/error.h"
> #include "qemu/target-info-qapi.h"
> #include "system/igvm.h"
>+#include "glib.h"

is this needed?

> #include "system/memory.h"
> #include "system/address-spaces.h"
> #include "hw/core/cpu.h"
>+#include "hw/i386/acpi-build.h"
>
> #include "trace.h"
>
>@@ -134,6 +136,8 @@ static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data,
> static int qigvm_initialization_guest_policy(QIgvm *ctx,
>                                        const uint8_t *header_data,
>                                        Error **errp);
>+static int qigvm_directive_madt(QIgvm *ctx,
>+                                     const uint8_t *header_data, Error **errp);
>
> struct QIGVMHandler {
>     uint32_t type;
>@@ -162,6 +166,8 @@ static struct QIGVMHandler handlers[] = {
>       qigvm_directive_snp_id_block },
>     { IGVM_VHT_GUEST_POLICY, IGVM_HEADER_SECTION_INITIALIZATION,
>       qigvm_initialization_guest_policy },
>+    { IGVM_VHT_MADT, IGVM_HEADER_SECTION_DIRECTIVE,
>+      qigvm_directive_madt },
> };
>
> static int qigvm_handler(QIgvm *ctx, uint32_t type, Error **errp)
>@@ -780,6 +786,35 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
>     return 0;
> }
>
>+static int qigvm_directive_madt(QIgvm *ctx,
>+                                     const uint8_t *header_data, Error **errp)
>+{
>+    const IGVM_VHS_PARAMETER *param = (const IGVM_VHS_PARAMETER *)header_data;
>+    QIgvmParameterData *param_entry;
>+
>+    if (ctx->machine_state == NULL) {
>+        return 0;
>+    }
>+
>+    /* Find the parameter area that should hold the MADT data */
>+    param_entry = qigvm_find_param_entry(ctx, param);
>+    if (param_entry != NULL) {
>+
>+        GArray *madt = acpi_build_madt_standalone(ctx->machine_state);
>+
>+        if (madt->len > param_entry->size) {
>+            error_setg(
>+                errp,
>+                "IGVM: MADT size exceeds parameter area defined in IGVM file");
>+            return -1;
>+        }
>+        memcpy(param_entry->data, madt->data, madt->len);
>+
>+        g_array_free(madt, true);
>+    }
>+    return 0;
>+}
>+
> static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp)
> {
>     int32_t header_count;
>--
>2.52.0
>

Rest LGTM!

Luigi