Re: [PATCH 2/4] igvm: move set_id_block call into the SNP ID block directive handler

[Stefano Garzarella <sgarzare@redhat.com>]

On Tue, Sep 01, 2026 at 12:09:19PM +0200, Luigi Leonardi wrote:
>set_id_block only makes sense when the IGVM file contains an
>IGVM_VHT_SNP_ID_BLOCK directive. Move the call from the removed
>qigvm_handle_policy into qigvm_directive_snp_id_block, where the ID
>block and ID auth are populated. This avoids a no-op call to
>set_id_block when no ID block is present.
>
>The ID block embeds the guest policy, so the policy must be known by the
>time the directive is handled. Process the initialization section (which
>carries the GUEST_POLICY header) before the directive section, and
>copy ctx->sev_policy into the ID block in the directive handler.
>
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> backends/igvm.c | 81 +++++++++++++++++++++++++++------------------------------
> 1 file changed, 38 insertions(+), 43 deletions(-)
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 85de0d54ec..6545382546 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -778,6 +778,8 @@ static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data,
>     ctx->id_block->version = IGVM_SEV_ID_BLOCK_VERSION;
>     memcpy(ctx->id_block->ld, igvm_id->ld, sizeof(ctx->id_block->ld));
>
>+    ctx->id_block->policy = ctx->sev_policy;

Is it fine to copy the sev_policy in the id_block in any case?

I mean, what happen if IGVM_VHT_GUEST_POLICY is not in the IGVM file, so 
IIUC sev_policy is 0, but the user set the policy by the CLI?

Maybe this was pre-existing and handled in the next patches.

Thanks,
Stefano

>+
>     ctx->id_auth->id_key_alg = igvm_id->id_key_algorithm;
>     assert(sizeof(igvm_id->id_key_signature) <=
>            sizeof(ctx->id_auth->id_block_sig));
>@@ -805,6 +807,14 @@ static int qigvm_directive_snp_id_block(QIgvm *ctx, const uint8_t *header_data,
>     memcpy(&ctx->id_auth->author_key[76], &igvm_id->author_public_key.qy,
>             72);
>
>+    if (ctx->cgsc) {
>+        return ctx->cgsc->set_id_block(ctx->id_block,
>+                                       sizeof(struct sev_id_block),
>+                                       ctx->id_auth,
>+                                       sizeof(struct sev_id_authentication),
>+                                       errp);
>+    }
>+
>     return 0;
> }
>
>@@ -958,23 +968,6 @@ static int qigvm_supported_platform_compat_mask(QIgvm *ctx, Error **errp)
>     return 0;
> }
>
>-static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
>-{
>-    if (ctx->platform_type == IGVM_PLATFORM_TYPE_SEV_SNP) {
>-        int id_block_len = 0;
>-        int id_auth_len = 0;
>-        if (ctx->id_block) {
>-            ctx->id_block->policy = ctx->sev_policy;
>-            id_block_len = sizeof(struct sev_id_block);
>-            id_auth_len = sizeof(struct sev_id_authentication);
>-        }
>-
>-        return ctx->cgsc->set_id_block(ctx->id_block, id_block_len,
>-                                       ctx->id_auth, id_auth_len, errp);
>-    }
>-    return 0;
>-}
>-
> IgvmHandle qigvm_file_init(char *filename, Error **errp)
> {
>     IgvmHandle igvm;
>@@ -1032,6 +1025,34 @@ int qigvm_process_file(IgvmCfg *cfg, MachineState *machine_state,
>         goto cleanup;
>     }
>
>+    /*
>+     * Process the initialization section first so that the guest policy is
>+     * known before the directive section is handled. The SNP ID block
>+     * directive embeds the guest policy into the ID block, so the policy from
>+     * the guest policy initialization header must be available by then.
>+     */
>+    header_count =
>+        igvm_header_count(ctx.cfg->file, IGVM_HEADER_SECTION_INITIALIZATION);
>+    if (header_count < 0) {
>+        error_setg(
>+            errp,
>+            "Invalid initialization header count in IGVM file. Error code: %X",
>+            header_count);
>+        goto cleanup;
>+    }
>+
>+    for (ctx.current_header_index = 0;
>+         ctx.current_header_index < (unsigned)header_count;
>+         ctx.current_header_index++) {
>+        IgvmVariableHeaderType type =
>+            igvm_get_header_type(ctx.cfg->file,
>+                                 IGVM_HEADER_SECTION_INITIALIZATION,
>+                                 ctx.current_header_index);
>+        if (qigvm_handler(&ctx, type, errp) < 0) {
>+            goto cleanup;
>+        }
>+    }
>+
>     header_count = igvm_header_count(ctx.cfg->file,
>                                      IGVM_HEADER_SECTION_DIRECTIVE);
>     if (header_count <= 0) {
>@@ -1065,28 +1086,6 @@ int qigvm_process_file(IgvmCfg *cfg, MachineState *machine_state,
>         goto cleanup_parameters;
>     }
>
>-    header_count =
>-        igvm_header_count(ctx.cfg->file, IGVM_HEADER_SECTION_INITIALIZATION);
>-    if (header_count < 0) {
>-        error_setg(
>-            errp,
>-            "Invalid initialization header count in IGVM file. Error code: %X",
>-            header_count);
>-        goto cleanup_parameters;
>-    }
>-
>-    for (ctx.current_header_index = 0;
>-         ctx.current_header_index < (unsigned)header_count;
>-         ctx.current_header_index++) {
>-        IgvmVariableHeaderType type =
>-            igvm_get_header_type(ctx.cfg->file,
>-                                 IGVM_HEADER_SECTION_INITIALIZATION,
>-                                 ctx.current_header_index);
>-        if (qigvm_handler(&ctx, type, errp) < 0) {
>-            goto cleanup_parameters;
>-        }
>-    }
>-
>     /*
>      * Contiguous pages of data with compatible flags are grouped together in
>      * order to reduce the number of memory regions we create. Make sure the
>@@ -1094,10 +1093,6 @@ int qigvm_process_file(IgvmCfg *cfg, MachineState *machine_state,
>      */
>     retval = qigvm_process_mem_page(&ctx, NULL, errp);
>
>-    if (retval == 0) {
>-        retval = qigvm_handle_policy(&ctx, errp);
>-    }
>-
> cleanup_parameters:
>     QTAILQ_FOREACH(parameter, &ctx.parameter_data, next)
>     {
>
>-- 
>2.55.0
>