[PATCH 0/4] igvm/sev: apply the IGVM guest policy before launch

[PATCH 0/4] igvm/sev: apply the IGVM guest policy before launch

[Luigi Leonardi]

The guest policy from an IGVM file never actually made it to the platform
before launch. The callback that was supposed to forward it ran after
LAUNCH_START, so it did nothing and the guest was launched with the
default policy instead of the one requested by the file.

The policy is part of the attestation report, so this quietly breaks
attestation: the resulting measurement does not match what the IGVM file
was built for.

Patches 1 to 3 are cleanups needed to get there: dropping an overloaded
callback, moving the SNP ID block handling and giving the policy
properties proper accessors. Patch 4 is the actual fix.

One thing worth calling out: if a policy is also passed on the command
line and it differs from the IGVM one, we now return an error instead of
silently picking one over the other.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
Luigi Leonardi (4):
      sev: rename set_guest_policy to set_id_block and remove dead policy code
      igvm: move set_id_block call into the SNP ID block directive handler
      i386/sev: convert the guest policy properties to custom accessors
      igvm/sev: forward the IGVM guest policy to the platform before launch

 backends/confidential-guest-support.c       |  17 ++-
 backends/igvm.c                             |  85 +++++++------
 include/system/confidential-guest-support.h |  28 +++--
 target/i386/sev.c                           | 181 ++++++++++++++++------------
 4 files changed, 174 insertions(+), 137 deletions(-)
---
base-commit: d2e570cc0f97b936902a5b1b86b73c0f5998b475
change-id: 20260831-fix_igvm_policy-0b92de52dd6a

Best regards,
-- 
Luigi Leonardi <leonardi@redhat.com>

[PATCH 1/4] sev: rename set_guest_policy to set_id_block and remove dead policy code

[Luigi Leonardi]

The guest policy must be provided at guest launch start: LAUNCH_START for
SEV/SEV-ES and SNP_LAUNCH_START for SEV-SNP. See the SEV API
specification, chapter 3 (Guest Policy), and the SEV-SNP firmware ABI
specification, section 4.3 (Guest Policy).

The policy parameter in set_guest_policy was never effective: by the
time this callback runs, LAUNCH_START has already been issued for both
SEV/SEV-ES and SEV-SNP, so writing to kvm_start_conf.policy or
sev_guest->policy has no effect. In practice the only thing this
callback actually does is set the ID block and ID auth for SNP's
LAUNCH_FINISH, so rename it to set_id_block to reflect its real
purpose, remove the unused policy parameter, and drop the non-SNP code
path which was entirely dead.

This is preliminary work: actually forwarding the guest policy to the
platform before LAUNCH_START is added in a later commit.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 backends/confidential-guest-support.c       |  12 ++-
 backends/igvm.c                             |   6 +-
 include/system/confidential-guest-support.h |  28 ++++---
 target/i386/sev.c                           | 118 +++++++++++-----------------
 4 files changed, 67 insertions(+), 97 deletions(-)

diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
index 156dd15e66..a0b36d2da5 100644
--- a/backends/confidential-guest-support.c
+++ b/backends/confidential-guest-support.c
@@ -38,14 +38,12 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
     return -1;
 }
 
-static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
-                            uint64_t policy,
-                            void *policy_data1, uint32_t policy_data1_size,
-                            void *policy_data2, uint32_t policy_data2_size,
-                            Error **errp)
+static int set_id_block(void *id_block, uint32_t id_block_size,
+                        void *id_auth, uint32_t id_auth_size,
+                        Error **errp)
 {
     error_setg(errp,
-               "Setting confidential guest policy is not supported for this platform");
+               "Setting ID block is not supported for this platform");
     return -1;
 }
 
@@ -64,7 +62,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
     ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
     cgsc->check_support = check_support;
     cgsc->set_guest_state = set_guest_state;
-    cgsc->set_guest_policy = set_guest_policy;
+    cgsc->set_id_block = set_id_block;
     cgsc->get_mem_map_entry = get_mem_map_entry;
 }
 
diff --git a/backends/igvm.c b/backends/igvm.c
index 7b7bdc72b7..85de0d54ec 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -968,9 +968,9 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
             id_block_len = sizeof(struct sev_id_block);
             id_auth_len = sizeof(struct sev_id_authentication);
         }
-        return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV, ctx->sev_policy,
-                                          ctx->id_block, id_block_len,
-                                          ctx->id_auth, id_auth_len, errp);
+
+        return ctx->cgsc->set_id_block(ctx->id_block, id_block_len,
+                                       ctx->id_auth, id_auth_len, errp);
     }
     return 0;
 }
diff --git a/include/system/confidential-guest-support.h b/include/system/confidential-guest-support.h
index 5dca717308..6d35ddb97a 100644
--- a/include/system/confidential-guest-support.h
+++ b/include/system/confidential-guest-support.h
@@ -128,21 +128,23 @@ typedef struct ConfidentialGuestSupportClass {
                            uint16_t cpu_index, Error **errp);
 
     /*
-     * Set the guest policy. The policy can be used to configure the
-     * confidential platform, such as if debug is enabled or not and can contain
-     * information about expected launch measurements, signed verification of
-     * guest configuration and other platform data.
-     *
-     * The format of the policy data is specific to each platform. For example,
-     * SEV-SNP uses a policy bitfield in the 'policy' argument and provides an
-     * ID block and ID authentication in the 'policy_data' parameters. The type
-     * of policy data is identified by the 'policy_type' argument.
+     * Set the guest policy for the confidential platform. The policy
+     * configures properties of the guest, such as whether debug is
+     * enabled. Its format is platform-specific; for SEV/SEV-ES and
+     * SEV-SNP it is a policy bitfield. Must be called before LAUNCH_START
+     * so the policy is in effect for launch.
      */
     int (*set_guest_policy)(ConfidentialGuestPolicyType policy_type,
-                            uint64_t policy,
-                            void *policy_data1, uint32_t policy_data1_size,
-                            void *policy_data2, uint32_t policy_data2_size,
-                            Error **errp);
+                            uint64_t policy, Error **errp);
+
+    /*
+     * Set the SEV-SNP ID block and ID authentication block. These are
+     * passed to SNP_LAUNCH_FINISH to provide signed verification of the
+     * guest configuration.
+     */
+    int (*set_id_block)(void *id_block, uint32_t id_block_size,
+                        void *id_auth, uint32_t id_auth_size,
+                        Error **errp);
 
     /*
      * Iterate the system memory map, getting the entry with the given index
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 4d875d10ff..465415c535 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -2723,10 +2723,9 @@ static int cgs_get_mem_map_entry(int index,
     return 0;
 }
 
-static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
-                                uint64_t policy, void *policy_data1,
-                                uint32_t policy_data1_size, void *policy_data2,
-                                uint32_t policy_data2_size, Error **errp)
+static int cgs_set_id_block(void *id_block, uint32_t id_block_size,
+                            void *id_auth, uint32_t id_auth_size,
+                            Error **errp)
 {
     SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
     if (sev_common->state == SEV_STATE_UNINIT) {
@@ -2734,86 +2733,57 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
         return 0;
     }
 
-    if (policy_type != GUEST_POLICY_SEV) {
-        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
-                   policy_type);
+    if (!sev_snp_enabled()) {
+        error_setg(errp, "SEV: ID block is only supported for SEV-SNP");
         return -1;
     }
-    /*
-     * SEV-SNP handles policy differently. The policy flags are defined in
-     * kvm_start_conf.policy and an ID block and ID auth can be provided.
-     */
-    if (sev_snp_enabled()) {
-        SevSnpGuestState *sev_snp_guest =
-            SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
-        struct kvm_sev_snp_launch_finish *finish =
-            &sev_snp_guest->kvm_finish_conf;
 
-        /*
-         * The policy consists of flags in 'policy' and optionally an ID block
-         * and ID auth in policy_data1 and policy_data2 respectively. The ID
-         * block and auth are optional so clear any previous ID block and auth
-         * and set them if provided, but always set the policy flags.
-         */
-        g_free(sev_snp_guest->id_block);
-        g_free((guchar *)finish->id_block_uaddr);
-        g_free(sev_snp_guest->id_auth);
-        g_free((guchar *)finish->id_auth_uaddr);
-        sev_snp_guest->id_block = NULL;
-        finish->id_block_uaddr = 0;
-        sev_snp_guest->id_auth = NULL;
-        finish->id_auth_uaddr = 0;
-
-        if (policy_data1_size > 0) {
-            struct sev_snp_id_authentication *id_auth =
-                (struct sev_snp_id_authentication *)policy_data2;
-
-            if (policy_data1_size != KVM_SEV_SNP_ID_BLOCK_SIZE) {
-                error_setg(errp, "SEV: Invalid SEV-SNP ID block: incorrect size");
-                return -1;
-            }
-            if (policy_data2_size != KVM_SEV_SNP_ID_AUTH_SIZE) {
-                error_setg(errp,
-                           "SEV: Invalid SEV-SNP ID auth block: incorrect size");
-                return -1;
-            }
-            assert(policy_data1 != NULL);
-            assert(policy_data2 != NULL);
+    SevSnpGuestState *sev_snp_guest =
+        SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
+    struct kvm_sev_snp_launch_finish *finish =
+        &sev_snp_guest->kvm_finish_conf;
 
-            finish->id_block_uaddr =
-                (__u64)g_memdup2(policy_data1, KVM_SEV_SNP_ID_BLOCK_SIZE);
-            finish->id_auth_uaddr =
-                (__u64)g_memdup2(policy_data2, KVM_SEV_SNP_ID_AUTH_SIZE);
+    g_free(sev_snp_guest->id_block);
+    g_free((guchar *)finish->id_block_uaddr);
+    g_free(sev_snp_guest->id_auth);
+    g_free((guchar *)finish->id_auth_uaddr);
+    sev_snp_guest->id_block = NULL;
+    finish->id_block_uaddr = 0;
+    sev_snp_guest->id_auth = NULL;
+    finish->id_auth_uaddr = 0;
 
-            /*
-             * Check if an author key has been provided and use that to flag
-             * whether the author key is enabled. The first of the author key
-             * must be non-zero to indicate the key type, which will currently
-             * always be 2.
-             */
-            sev_snp_guest->kvm_finish_conf.auth_key_en =
-                id_auth->author_key[0] ? 1 : 0;
-            finish->id_block_en = 1;
-        }
+    if (id_block_size > 0) {
+        struct sev_snp_id_authentication *auth =
+            (struct sev_snp_id_authentication *)id_auth;
 
-        /* do not reset existing policy if policy was not set in IGVM  */
-        if (policy != 0) {
-            sev_snp_guest->kvm_start_conf.policy = policy;
+        if (id_block_size != KVM_SEV_SNP_ID_BLOCK_SIZE) {
+            error_setg(errp, "SEV: Invalid SEV-SNP ID block: incorrect size");
+            return -1;
         }
-    } else {
-        SevGuestState *sev_guest = SEV_GUEST(MACHINE(qdev_get_machine())->cgs);
-        /* Only the policy flags are supported for SEV and SEV-ES */
-        if ((policy_data1_size > 0) || (policy_data2_size > 0) || !sev_guest) {
-            error_setg(errp, "SEV: An ID block/ID auth block has been provided "
-                             "but SEV-SNP is not enabled");
+        if (id_auth_size != KVM_SEV_SNP_ID_AUTH_SIZE) {
+            error_setg(errp,
+                       "SEV: Invalid SEV-SNP ID auth block: incorrect size");
             return -1;
         }
+        assert(id_block != NULL);
+        assert(id_auth != NULL);
 
-        /* do not reset existing policy if policy was not set in IGVM  */
-        if (policy != 0) {
-            sev_guest->policy = policy;
-        }
+        finish->id_block_uaddr =
+            (__u64)g_memdup2(id_block, KVM_SEV_SNP_ID_BLOCK_SIZE);
+        finish->id_auth_uaddr =
+            (__u64)g_memdup2(id_auth, KVM_SEV_SNP_ID_AUTH_SIZE);
+
+        /*
+         * Check if an author key has been provided and use that to flag
+         * whether the author key is enabled. The first of the author key
+         * must be non-zero to indicate the key type, which will currently
+         * always be 2.
+         */
+        sev_snp_guest->kvm_finish_conf.auth_key_en =
+            auth->author_key[0] ? 1 : 0;
+        finish->id_block_en = 1;
     }
+
     return 0;
 }
 
@@ -2878,7 +2848,7 @@ sev_common_instance_init(Object *obj)
     cgs->check_support = cgs_check_support;
     cgs->set_guest_state = cgs_set_guest_state;
     cgs->get_mem_map_entry = cgs_get_mem_map_entry;
-    cgs->set_guest_policy = cgs_set_guest_policy;
+    cgs->set_id_block = cgs_set_id_block;
     cgs->can_rebuild_guest_state = true;
 
     QTAILQ_INIT(&sev_common->launch_vmsa);

-- 
2.55.0

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

[Luigi Leonardi]

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;
+
     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

[PATCH 3/4] i386/sev: convert the guest policy properties to custom accessors

[Luigi Leonardi]

Both SEV/SEV-ES and SEV-SNP expose a "policy" object property. The
SEV/SEV-ES one was registered as a plain uint32 pointer property, and
the SEV-SNP setter ignored the result of the visit.

Give both properties explicit getter/setter functions and check the
return value of the visit in the setters. This is preparation for
tracking whether the guest policy was set on the command line.

No functional change intended.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 target/i386/sev.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 465415c535..c76cdba8d2 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -2956,6 +2956,22 @@ sev_guest_class_init(ObjectClass *oc, const void *data)
             "use legacy VM type to maintain measurement compatibility with older QEMU or kernel versions.");
 }
 
+static void
+sev_guest_get_policy(Object *obj, Visitor *v, const char *name,
+                     void *opaque, Error **errp)
+{
+    visit_type_uint32(v, name, &SEV_GUEST(obj)->policy, errp);
+}
+
+static void
+sev_guest_set_policy(Object *obj, Visitor *v, const char *name,
+                     void *opaque, Error **errp)
+{
+    if (!visit_type_uint32(v, name, &SEV_GUEST(obj)->policy, errp)) {
+        return;
+    }
+}
+
 static void
 sev_guest_instance_init(Object *obj)
 {
@@ -2964,8 +2980,8 @@ sev_guest_instance_init(Object *obj)
     sev_guest->policy = DEFAULT_GUEST_POLICY;
     object_property_add_uint32_ptr(obj, "handle", &sev_guest->handle,
                                    OBJ_PROP_FLAG_READWRITE);
-    object_property_add_uint32_ptr(obj, "policy", &sev_guest->policy,
-                                   OBJ_PROP_FLAG_READWRITE);
+    object_property_add(obj, "policy", "uint32", sev_guest_get_policy,
+                        sev_guest_set_policy, NULL, NULL);
     object_apply_compat_props(obj);
 
     sev_guest->legacy_vm_type = ON_OFF_AUTO_AUTO;
@@ -3004,9 +3020,13 @@ static void
 sev_snp_guest_set_policy(Object *obj, Visitor *v, const char *name,
                          void *opaque, Error **errp)
 {
-    visit_type_uint64(v, name,
-                      (uint64_t *)&SEV_SNP_GUEST(obj)->kvm_start_conf.policy,
-                      errp);
+    SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
+
+    if (!visit_type_uint64(v, name,
+                           (uint64_t *)&sev_snp_guest->kvm_start_conf.policy,
+                           errp)) {
+        return;
+    }
 }
 
 static char *

-- 
2.55.0

[PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Luigi Leonardi]

The guest policy carried in the IGVM guest-policy initialization header
was parsed into QIgvm but never forwarded to the confidential guest
platform: the previous callback ran at the end of qigvm_process_file,
after LAUNCH_START had already been issued, so writing the policy had
no effect.

Add a set_guest_policy callback and invoke it from the guest-policy
initialization handler, so the policy reaches the platform before
LAUNCH_START.

The guest policy can also be set on the command line. As it is part of
the attestation report, silently overriding it would cause attestation
to fail, so return an error if the command-line value differs from the
one supplied by the IGVM file.

Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 backends/confidential-guest-support.c |  9 ++++++++
 backends/igvm.c                       |  4 ++++
 target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
index a0b36d2da5..c0d15b4a76 100644
--- a/backends/confidential-guest-support.c
+++ b/backends/confidential-guest-support.c
@@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
     return -1;
 }
 
+static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
+                            uint64_t policy, Error **errp)
+{
+    error_setg(errp,
+               "Setting guest policy is not supported for this platform");
+    return -1;
+}
+
 static int set_id_block(void *id_block, uint32_t id_block_size,
                         void *id_auth, uint32_t id_auth_size,
                         Error **errp)
@@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
     ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
     cgsc->check_support = check_support;
     cgsc->set_guest_state = set_guest_state;
+    cgsc->set_guest_policy = set_guest_policy;
     cgsc->set_id_block = set_id_block;
     cgsc->get_mem_map_entry = get_mem_map_entry;
 }
diff --git a/backends/igvm.c b/backends/igvm.c
index 6545382546..5131ee7829 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
 
     if (guest->compatibility_mask & ctx->compatibility_mask) {
         ctx->sev_policy = guest->policy;
+        if (ctx->cgsc) {
+            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
+                                               guest->policy, errp);
+        }
     }
     return 0;
 }
diff --git a/target/i386/sev.c b/target/i386/sev.c
index c76cdba8d2..533ea4b54e 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -128,6 +128,8 @@ struct SevCommonState {
     bool kernel_hashes;
     uint64_t sev_features;
     uint64_t supported_sev_features;
+    /* whether the guest policy was explicitly set on the command line */
+    bool policy_set;
 
     /* runtime state */
     uint8_t api_major;
@@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
     return 0;
 }
 
+static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
+                                uint64_t policy, Error **errp)
+{
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+
+    if (policy_type != GUEST_POLICY_SEV) {
+        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
+                   policy_type);
+        return -1;
+    }
+
+    if (sev_snp_enabled()) {
+        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
+
+        if (sev_common->policy_set &&
+            sev_snp_guest->kvm_start_conf.policy != policy) {
+            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
+            return -1;
+        }
+
+        sev_snp_guest->kvm_start_conf.policy = policy;
+    } else {
+        SevGuestState *sev_guest = SEV_GUEST(sev_common);
+
+        if (sev_common->policy_set && sev_guest->policy != policy) {
+            error_setg(errp, "SEV: policy mismatch between IGVM and CLI");
+            return -1;
+        }
+
+        sev_guest->policy = policy;
+    }
+    return 0;
+}
+
 static int cgs_set_id_block(void *id_block, uint32_t id_block_size,
                             void *id_auth, uint32_t id_auth_size,
                             Error **errp)
@@ -2848,6 +2884,7 @@ sev_common_instance_init(Object *obj)
     cgs->check_support = cgs_check_support;
     cgs->set_guest_state = cgs_set_guest_state;
     cgs->get_mem_map_entry = cgs_get_mem_map_entry;
+    cgs->set_guest_policy = cgs_set_guest_policy;
     cgs->set_id_block = cgs_set_id_block;
     cgs->can_rebuild_guest_state = true;
 
@@ -2970,6 +3007,7 @@ sev_guest_set_policy(Object *obj, Visitor *v, const char *name,
     if (!visit_type_uint32(v, name, &SEV_GUEST(obj)->policy, errp)) {
         return;
     }
+    SEV_COMMON(obj)->policy_set = true;
 }
 
 static void
@@ -3027,6 +3065,7 @@ sev_snp_guest_set_policy(Object *obj, Visitor *v, const char *name,
                            errp)) {
         return;
     }
+    SEV_COMMON(obj)->policy_set = true;
 }
 
 static char *

-- 
2.55.0

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Ani Sinha]

> On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> 
> The guest policy carried in the IGVM guest-policy initialization header
> was parsed into QIgvm but never forwarded to the confidential guest
> platform: the previous callback ran at the end of qigvm_process_file,
> after LAUNCH_START had already been issued, so writing the policy had
> no effect.
> 
> Add a set_guest_policy callback and invoke it from the guest-policy
> initialization handler, so the policy reaches the platform before
> LAUNCH_START.
> 
> The guest policy can also be set on the command line. As it is part of
> the attestation report, silently overriding it would cause attestation
> to fail, so return an error if the command-line value differs from the
> one supplied by the IGVM file.
> 
> Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
> Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> ---
> backends/confidential-guest-support.c |  9 ++++++++
> backends/igvm.c                       |  4 ++++
> target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+)
> 
> diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
> index a0b36d2da5..c0d15b4a76 100644
> --- a/backends/confidential-guest-support.c
> +++ b/backends/confidential-guest-support.c
> @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>     return -1;
> }
> 
> +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
> +                            uint64_t policy, Error **errp)
> +{
> +    error_setg(errp,
> +               "Setting guest policy is not supported for this platform");
> +    return -1;
> +}
> +
> static int set_id_block(void *id_block, uint32_t id_block_size,
>                         void *id_auth, uint32_t id_auth_size,
>                         Error **errp)
> @@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>     ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>     cgsc->check_support = check_support;
>     cgsc->set_guest_state = set_guest_state;
> +    cgsc->set_guest_policy = set_guest_policy;
>     cgsc->set_id_block = set_id_block;
>     cgsc->get_mem_map_entry = get_mem_map_entry;
> }
> diff --git a/backends/igvm.c b/backends/igvm.c
> index 6545382546..5131ee7829 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
> 
>     if (guest->compatibility_mask & ctx->compatibility_mask) {
>         ctx->sev_policy = guest->policy;
> +        if (ctx->cgsc) {
> +            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
> +                                               guest->policy, errp);
> +        }
>     }
>     return 0;
> }
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index c76cdba8d2..533ea4b54e 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -128,6 +128,8 @@ struct SevCommonState {
>     bool kernel_hashes;
>     uint64_t sev_features;
>     uint64_t supported_sev_features;
> +    /* whether the guest policy was explicitly set on the command line */
> +    bool policy_set;
> 
>     /* runtime state */
>     uint8_t api_major;
> @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>     return 0;
> }
> 
> +static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
> +                                uint64_t policy, Error **errp)
> +{
> +    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> +
> +    if (policy_type != GUEST_POLICY_SEV) {
> +        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
> +                   policy_type);
> +        return -1;
> +    }
> +
> +    if (sev_snp_enabled()) {
> +        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
> +
> +        if (sev_common->policy_set &&
> +            sev_snp_guest->kvm_start_conf.policy != policy) {
> +            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
> +            return -1;
> +        }
> +
> +        sev_snp_guest->kvm_start_conf.policy = policy;

I had fixed a bug initially here where we need to check if the policy passed is not 0. I am not sure if that fix is still needed here.
Please test this scenario:
a) Generate an IGVM file with policy set to 0.
b) Start a confidential SEV-SNP guest with policy set in command line and with this IGVM.
I think with your patch this will fail as the policies won’t match. This potentially breaks some tests. For example, with your patch, does my FUKI confidential tests still pass?
c) If b) works, reset the guest and check if the guest is still a SEV-SNP guest.
 

> +    } else {
> +        SevGuestState *sev_guest = SEV_GUEST(sev_common);
> +
> +        if (sev_common->policy_set && sev_guest->policy != policy) {
> +            error_setg(errp, "SEV: policy mismatch between IGVM and CLI");
> +            return -1;
> +        }
> +
> +        sev_guest->policy = policy;
> +    }
> +    return 0;
> +}
> +
> static int cgs_set_id_block(void *id_block, uint32_t id_block_size,
>                             void *id_auth, uint32_t id_auth_size,
>                             Error **errp)
> @@ -2848,6 +2884,7 @@ sev_common_instance_init(Object *obj)
>     cgs->check_support = cgs_check_support;
>     cgs->set_guest_state = cgs_set_guest_state;
>     cgs->get_mem_map_entry = cgs_get_mem_map_entry;
> +    cgs->set_guest_policy = cgs_set_guest_policy;
>     cgs->set_id_block = cgs_set_id_block;
>     cgs->can_rebuild_guest_state = true;
> 
> @@ -2970,6 +3007,7 @@ sev_guest_set_policy(Object *obj, Visitor *v, const char *name,
>     if (!visit_type_uint32(v, name, &SEV_GUEST(obj)->policy, errp)) {
>         return;
>     }
> +    SEV_COMMON(obj)->policy_set = true;
> }
> 
> static void
> @@ -3027,6 +3065,7 @@ sev_snp_guest_set_policy(Object *obj, Visitor *v, const char *name,
>                            errp)) {
>         return;
>     }
> +    SEV_COMMON(obj)->policy_set = true;
> }
> 
> static char *
> 
> -- 
> 2.55.0
> 

Re: [PATCH 3/4] i386/sev: convert the guest policy properties to custom accessors

[Ani Sinha]

> On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> 
> Both SEV/SEV-ES and SEV-SNP expose a "policy" object property. The
> SEV/SEV-ES one was registered as a plain uint32 pointer property, and
> the SEV-SNP setter ignored the result of the visit.
> 
> Give both properties explicit getter/setter functions and check the
> return value of the visit in the setters. This is preparation for
> tracking whether the guest policy was set on the command line.
> 
> No functional change intended.
> 
> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> target/i386/sev.c | 30 +++++++++++++++++++++++++-----
> 1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 465415c535..c76cdba8d2 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -2956,6 +2956,22 @@ sev_guest_class_init(ObjectClass *oc, const void *data)
>             "use legacy VM type to maintain measurement compatibility with older QEMU or kernel versions.");
> }
> 
> +static void
> +sev_guest_get_policy(Object *obj, Visitor *v, const char *name,
> +                     void *opaque, Error **errp)
> +{
> +    visit_type_uint32(v, name, &SEV_GUEST(obj)->policy, errp);
> +}
> +
> +static void
> +sev_guest_set_policy(Object *obj, Visitor *v, const char *name,
> +                     void *opaque, Error **errp)
> +{
> +    if (!visit_type_uint32(v, name, &SEV_GUEST(obj)->policy, errp)) {
> +        return;
> +    }
> +}
> +
> static void
> sev_guest_instance_init(Object *obj)
> {
> @@ -2964,8 +2980,8 @@ sev_guest_instance_init(Object *obj)
>     sev_guest->policy = DEFAULT_GUEST_POLICY;
>     object_property_add_uint32_ptr(obj, "handle", &sev_guest->handle,
>                                    OBJ_PROP_FLAG_READWRITE);
> -    object_property_add_uint32_ptr(obj, "policy", &sev_guest->policy,
> -                                   OBJ_PROP_FLAG_READWRITE);
> +    object_property_add(obj, "policy", "uint32", sev_guest_get_policy,
> +                        sev_guest_set_policy, NULL, NULL);
>     object_apply_compat_props(obj);
> 
>     sev_guest->legacy_vm_type = ON_OFF_AUTO_AUTO;
> @@ -3004,9 +3020,13 @@ static void
> sev_snp_guest_set_policy(Object *obj, Visitor *v, const char *name,
>                          void *opaque, Error **errp)
> {
> -    visit_type_uint64(v, name,
> -                      (uint64_t *)&SEV_SNP_GUEST(obj)->kvm_start_conf.policy,
> -                      errp);
> +    SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
> +
> +    if (!visit_type_uint64(v, name,
> +                           (uint64_t *)&sev_snp_guest->kvm_start_conf.policy,
> +                           errp)) {
> +        return;
> +    }
> }
> 
> static char *
> 
> -- 
> 2.55.0
> 

Re: [PATCH 1/4] sev: rename set_guest_policy to set_id_block and remove dead policy code

[Ani Sinha]

> On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> 
> The guest policy must be provided at guest launch start: LAUNCH_START for
> SEV/SEV-ES and SNP_LAUNCH_START for SEV-SNP. See the SEV API
> specification, chapter 3 (Guest Policy), and the SEV-SNP firmware ABI
> specification, section 4.3 (Guest Policy).
> 
> The policy parameter in set_guest_policy was never effective: by the
> time this callback runs, LAUNCH_START has already been issued for both
> SEV/SEV-ES and SEV-SNP, so writing to kvm_start_conf.policy or
> sev_guest->policy has no effect. In practice the only thing this
> callback actually does is set the ID block and ID auth for SNP's
> LAUNCH_FINISH, so rename it to set_id_block to reflect its real
> purpose, remove the unused policy parameter, and drop the non-SNP code
> path which was entirely dead.
> 
> This is preliminary work: actually forwarding the guest policy to the
> platform before LAUNCH_START is added in a later commit.
> 
> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> backends/confidential-guest-support.c       |  12 ++-
> backends/igvm.c                             |   6 +-
> include/system/confidential-guest-support.h |  28 ++++---
> target/i386/sev.c                           | 118 +++++++++++-----------------
> 4 files changed, 67 insertions(+), 97 deletions(-)
> 
> diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
> index 156dd15e66..a0b36d2da5 100644
> --- a/backends/confidential-guest-support.c
> +++ b/backends/confidential-guest-support.c
> @@ -38,14 +38,12 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>     return -1;
> }
> 
> -static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
> -                            uint64_t policy,
> -                            void *policy_data1, uint32_t policy_data1_size,
> -                            void *policy_data2, uint32_t policy_data2_size,
> -                            Error **errp)
> +static int set_id_block(void *id_block, uint32_t id_block_size,
> +                        void *id_auth, uint32_t id_auth_size,
> +                        Error **errp)
> {
>     error_setg(errp,
> -               "Setting confidential guest policy is not supported for this platform");
> +               "Setting ID block is not supported for this platform");
>     return -1;
> }
> 
> @@ -64,7 +62,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>     ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>     cgsc->check_support = check_support;
>     cgsc->set_guest_state = set_guest_state;
> -    cgsc->set_guest_policy = set_guest_policy;
> +    cgsc->set_id_block = set_id_block;
>     cgsc->get_mem_map_entry = get_mem_map_entry;
> }
> 
> diff --git a/backends/igvm.c b/backends/igvm.c
> index 7b7bdc72b7..85de0d54ec 100644
> --- a/backends/igvm.c
> +++ b/backends/igvm.c
> @@ -968,9 +968,9 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
>             id_block_len = sizeof(struct sev_id_block);
>             id_auth_len = sizeof(struct sev_id_authentication);
>         }
> -        return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV, ctx->sev_policy,
> -                                          ctx->id_block, id_block_len,
> -                                          ctx->id_auth, id_auth_len, errp);
> +
> +        return ctx->cgsc->set_id_block(ctx->id_block, id_block_len,
> +                                       ctx->id_auth, id_auth_len, errp);
>     }
>     return 0;
> }
> diff --git a/include/system/confidential-guest-support.h b/include/system/confidential-guest-support.h
> index 5dca717308..6d35ddb97a 100644
> --- a/include/system/confidential-guest-support.h
> +++ b/include/system/confidential-guest-support.h
> @@ -128,21 +128,23 @@ typedef struct ConfidentialGuestSupportClass {
>                            uint16_t cpu_index, Error **errp);
> 
>     /*
> -     * Set the guest policy. The policy can be used to configure the
> -     * confidential platform, such as if debug is enabled or not and can contain
> -     * information about expected launch measurements, signed verification of
> -     * guest configuration and other platform data.
> -     *
> -     * The format of the policy data is specific to each platform. For example,
> -     * SEV-SNP uses a policy bitfield in the 'policy' argument and provides an
> -     * ID block and ID authentication in the 'policy_data' parameters. The type
> -     * of policy data is identified by the 'policy_type' argument.
> +     * Set the guest policy for the confidential platform. The policy
> +     * configures properties of the guest, such as whether debug is
> +     * enabled. Its format is platform-specific; for SEV/SEV-ES and
> +     * SEV-SNP it is a policy bitfield. Must be called before LAUNCH_START
> +     * so the policy is in effect for launch.
>      */
>     int (*set_guest_policy)(ConfidentialGuestPolicyType policy_type,
> -                            uint64_t policy,
> -                            void *policy_data1, uint32_t policy_data1_size,
> -                            void *policy_data2, uint32_t policy_data2_size,
> -                            Error **errp);
> +                            uint64_t policy, Error **errp);
> +
> +    /*
> +     * Set the SEV-SNP ID block and ID authentication block. These are
> +     * passed to SNP_LAUNCH_FINISH to provide signed verification of the
> +     * guest configuration.
> +     */
> +    int (*set_id_block)(void *id_block, uint32_t id_block_size,
> +                        void *id_auth, uint32_t id_auth_size,
> +                        Error **errp);
> 
>     /*
>      * Iterate the system memory map, getting the entry with the given index
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 4d875d10ff..465415c535 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -2723,10 +2723,9 @@ static int cgs_get_mem_map_entry(int index,
>     return 0;
> }
> 
> -static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
> -                                uint64_t policy, void *policy_data1,
> -                                uint32_t policy_data1_size, void *policy_data2,
> -                                uint32_t policy_data2_size, Error **errp)
> +static int cgs_set_id_block(void *id_block, uint32_t id_block_size,
> +                            void *id_auth, uint32_t id_auth_size,
> +                            Error **errp)
> {
>     SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>     if (sev_common->state == SEV_STATE_UNINIT) {
> @@ -2734,86 +2733,57 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>         return 0;
>     }
> 
> -    if (policy_type != GUEST_POLICY_SEV) {
> -        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
> -                   policy_type);
> +    if (!sev_snp_enabled()) {
> +        error_setg(errp, "SEV: ID block is only supported for SEV-SNP");
>         return -1;
>     }
> -    /*
> -     * SEV-SNP handles policy differently. The policy flags are defined in
> -     * kvm_start_conf.policy and an ID block and ID auth can be provided.
> -     */
> -    if (sev_snp_enabled()) {
> -        SevSnpGuestState *sev_snp_guest =
> -            SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
> -        struct kvm_sev_snp_launch_finish *finish =
> -            &sev_snp_guest->kvm_finish_conf;
> 
> -        /*
> -         * The policy consists of flags in 'policy' and optionally an ID block
> -         * and ID auth in policy_data1 and policy_data2 respectively. The ID
> -         * block and auth are optional so clear any previous ID block and auth
> -         * and set them if provided, but always set the policy flags.
> -         */
> -        g_free(sev_snp_guest->id_block);
> -        g_free((guchar *)finish->id_block_uaddr);
> -        g_free(sev_snp_guest->id_auth);
> -        g_free((guchar *)finish->id_auth_uaddr);
> -        sev_snp_guest->id_block = NULL;
> -        finish->id_block_uaddr = 0;
> -        sev_snp_guest->id_auth = NULL;
> -        finish->id_auth_uaddr = 0;
> -
> -        if (policy_data1_size > 0) {
> -            struct sev_snp_id_authentication *id_auth =
> -                (struct sev_snp_id_authentication *)policy_data2;
> -
> -            if (policy_data1_size != KVM_SEV_SNP_ID_BLOCK_SIZE) {
> -                error_setg(errp, "SEV: Invalid SEV-SNP ID block: incorrect size");
> -                return -1;
> -            }
> -            if (policy_data2_size != KVM_SEV_SNP_ID_AUTH_SIZE) {
> -                error_setg(errp,
> -                           "SEV: Invalid SEV-SNP ID auth block: incorrect size");
> -                return -1;
> -            }
> -            assert(policy_data1 != NULL);
> -            assert(policy_data2 != NULL);
> +    SevSnpGuestState *sev_snp_guest =
> +        SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
> +    struct kvm_sev_snp_launch_finish *finish =
> +        &sev_snp_guest->kvm_finish_conf;
> 
> -            finish->id_block_uaddr =
> -                (__u64)g_memdup2(policy_data1, KVM_SEV_SNP_ID_BLOCK_SIZE);
> -            finish->id_auth_uaddr =
> -                (__u64)g_memdup2(policy_data2, KVM_SEV_SNP_ID_AUTH_SIZE);
> +    g_free(sev_snp_guest->id_block);
> +    g_free((guchar *)finish->id_block_uaddr);
> +    g_free(sev_snp_guest->id_auth);
> +    g_free((guchar *)finish->id_auth_uaddr);
> +    sev_snp_guest->id_block = NULL;
> +    finish->id_block_uaddr = 0;
> +    sev_snp_guest->id_auth = NULL;
> +    finish->id_auth_uaddr = 0;
> 
> -            /*
> -             * Check if an author key has been provided and use that to flag
> -             * whether the author key is enabled. The first of the author key
> -             * must be non-zero to indicate the key type, which will currently
> -             * always be 2.
> -             */
> -            sev_snp_guest->kvm_finish_conf.auth_key_en =
> -                id_auth->author_key[0] ? 1 : 0;
> -            finish->id_block_en = 1;
> -        }
> +    if (id_block_size > 0) {
> +        struct sev_snp_id_authentication *auth =
> +            (struct sev_snp_id_authentication *)id_auth;
> 
> -        /* do not reset existing policy if policy was not set in IGVM  */
> -        if (policy != 0) {
> -            sev_snp_guest->kvm_start_conf.policy = policy;
> +        if (id_block_size != KVM_SEV_SNP_ID_BLOCK_SIZE) {
> +            error_setg(errp, "SEV: Invalid SEV-SNP ID block: incorrect size");
> +            return -1;
>         }
> -    } else {
> -        SevGuestState *sev_guest = SEV_GUEST(MACHINE(qdev_get_machine())->cgs);
> -        /* Only the policy flags are supported for SEV and SEV-ES */
> -        if ((policy_data1_size > 0) || (policy_data2_size > 0) || !sev_guest) {
> -            error_setg(errp, "SEV: An ID block/ID auth block has been provided "
> -                             "but SEV-SNP is not enabled");
> +        if (id_auth_size != KVM_SEV_SNP_ID_AUTH_SIZE) {
> +            error_setg(errp,
> +                       "SEV: Invalid SEV-SNP ID auth block: incorrect size");
>             return -1;
>         }
> +        assert(id_block != NULL);
> +        assert(id_auth != NULL);
> 
> -        /* do not reset existing policy if policy was not set in IGVM  */
> -        if (policy != 0) {
> -            sev_guest->policy = policy;
> -        }
> +        finish->id_block_uaddr =
> +            (__u64)g_memdup2(id_block, KVM_SEV_SNP_ID_BLOCK_SIZE);
> +        finish->id_auth_uaddr =
> +            (__u64)g_memdup2(id_auth, KVM_SEV_SNP_ID_AUTH_SIZE);
> +
> +        /*
> +         * Check if an author key has been provided and use that to flag
> +         * whether the author key is enabled. The first of the author key
> +         * must be non-zero to indicate the key type, which will currently
> +         * always be 2.
> +         */
> +        sev_snp_guest->kvm_finish_conf.auth_key_en =
> +            auth->author_key[0] ? 1 : 0;
> +        finish->id_block_en = 1;
>     }
> +
>     return 0;
> }
> 
> @@ -2878,7 +2848,7 @@ sev_common_instance_init(Object *obj)
>     cgs->check_support = cgs_check_support;
>     cgs->set_guest_state = cgs_set_guest_state;
>     cgs->get_mem_map_entry = cgs_get_mem_map_entry;
> -    cgs->set_guest_policy = cgs_set_guest_policy;
> +    cgs->set_id_block = cgs_set_id_block;
>     cgs->can_rebuild_guest_state = true;
> 
>     QTAILQ_INIT(&sev_common->launch_vmsa);
> 
> -- 
> 2.55.0
> 

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Luigi Leonardi]

Hi Ani,

On Thu, Sep 03, 2026 at 02:16:30PM +0530, Ani Sinha wrote:
>
>
>> On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>
>> The guest policy carried in the IGVM guest-policy initialization header
>> was parsed into QIgvm but never forwarded to the confidential guest
>> platform: the previous callback ran at the end of qigvm_process_file,
>> after LAUNCH_START had already been issued, so writing the policy had
>> no effect.
>>
>> Add a set_guest_policy callback and invoke it from the guest-policy
>> initialization handler, so the policy reaches the platform before
>> LAUNCH_START.
>>
>> The guest policy can also be set on the command line. As it is part of
>> the attestation report, silently overriding it would cause attestation
>> to fail, so return an error if the command-line value differs from the
>> one supplied by the IGVM file.
>>
>> Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
>> Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>> ---
>> backends/confidential-guest-support.c |  9 ++++++++
>> backends/igvm.c                       |  4 ++++
>> target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
>> 3 files changed, 52 insertions(+)
>>
>> diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>> index a0b36d2da5..c0d15b4a76 100644
>> --- a/backends/confidential-guest-support.c
>> +++ b/backends/confidential-guest-support.c
>> @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>>     return -1;
>> }
>>
>> +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>> +                            uint64_t policy, Error **errp)
>> +{
>> +    error_setg(errp,
>> +               "Setting guest policy is not supported for this platform");
>> +    return -1;
>> +}
>> +
>> static int set_id_block(void *id_block, uint32_t id_block_size,
>>                         void *id_auth, uint32_t id_auth_size,
>>                         Error **errp)
>> @@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>>     ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>>     cgsc->check_support = check_support;
>>     cgsc->set_guest_state = set_guest_state;
>> +    cgsc->set_guest_policy = set_guest_policy;
>>     cgsc->set_id_block = set_id_block;
>>     cgsc->get_mem_map_entry = get_mem_map_entry;
>> }
>> diff --git a/backends/igvm.c b/backends/igvm.c
>> index 6545382546..5131ee7829 100644
>> --- a/backends/igvm.c
>> +++ b/backends/igvm.c
>> @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
>>
>>     if (guest->compatibility_mask & ctx->compatibility_mask) {
>>         ctx->sev_policy = guest->policy;
>> +        if (ctx->cgsc) {
>> +            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
>> +                                               guest->policy, errp);
>> +        }
>>     }
>>     return 0;
>> }
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index c76cdba8d2..533ea4b54e 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -128,6 +128,8 @@ struct SevCommonState {
>>     bool kernel_hashes;
>>     uint64_t sev_features;
>>     uint64_t supported_sev_features;
>> +    /* whether the guest policy was explicitly set on the command line */
>> +    bool policy_set;
>>
>>     /* runtime state */
>>     uint8_t api_major;
>> @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>>     return 0;
>> }
>>
>> +static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>> +                                uint64_t policy, Error **errp)
>> +{
>> +    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>> +
>> +    if (policy_type != GUEST_POLICY_SEV) {
>> +        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
>> +                   policy_type);
>> +        return -1;
>> +    }
>> +
>> +    if (sev_snp_enabled()) {
>> +        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
>> +
>> +        if (sev_common->policy_set &&
>> +            sev_snp_guest->kvm_start_conf.policy != policy) {
>> +            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
>> +            return -1;
>> +        }
>> +
>> +        sev_snp_guest->kvm_start_conf.policy = policy;

Thanks for you review!

>
>I had fixed a bug initially here where we need to check if the policy passed is not 0. I am not sure if that fix is still needed here.
>Please test this scenario:
>a) Generate an IGVM file with policy set to 0.

0 is not a valid sev-snp guest policy: bit 17 is reserved and must be 1.
It's a valid sev/sev-es policy though.

>b) Start a confidential SEV-SNP guest with policy set in command line and with this IGVM.
>I think with your patch this will fail as the policies won’t match.

Correct: this was suggested by Gerd, as this is something unexpected. I
think it would break launch measurement.

>This potentially breaks some tests. For example, with your patch, does my FUKI confidential tests still pass?

To be fair, guest policy set via igvm _never_ worked. Only the policies
set from CLI worked.

That said, I'll try to run your tests.

Luigi

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Ani Sinha]

> On 3 Sep 2026, at 2:31 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> 
> Hi Ani,
> 
> On Thu, Sep 03, 2026 at 02:16:30PM +0530, Ani Sinha wrote:
>> 
>> 
>>> On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>> 
>>> The guest policy carried in the IGVM guest-policy initialization header
>>> was parsed into QIgvm but never forwarded to the confidential guest
>>> platform: the previous callback ran at the end of qigvm_process_file,
>>> after LAUNCH_START had already been issued, so writing the policy had
>>> no effect.
>>> 
>>> Add a set_guest_policy callback and invoke it from the guest-policy
>>> initialization handler, so the policy reaches the platform before
>>> LAUNCH_START.
>>> 
>>> The guest policy can also be set on the command line. As it is part of
>>> the attestation report, silently overriding it would cause attestation
>>> to fail, so return an error if the command-line value differs from the
>>> one supplied by the IGVM file.
>>> 
>>> Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
>>> Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
>>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>>> ---
>>> backends/confidential-guest-support.c |  9 ++++++++
>>> backends/igvm.c                       |  4 ++++
>>> target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
>>> 3 files changed, 52 insertions(+)
>>> 
>>> diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>>> index a0b36d2da5..c0d15b4a76 100644
>>> --- a/backends/confidential-guest-support.c
>>> +++ b/backends/confidential-guest-support.c
>>> @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>>>    return -1;
>>> }
>>> 
>>> +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>> +                            uint64_t policy, Error **errp)
>>> +{
>>> +    error_setg(errp,
>>> +               "Setting guest policy is not supported for this platform");
>>> +    return -1;
>>> +}
>>> +
>>> static int set_id_block(void *id_block, uint32_t id_block_size,
>>>                        void *id_auth, uint32_t id_auth_size,
>>>                        Error **errp)
>>> @@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>>>    ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>>>    cgsc->check_support = check_support;
>>>    cgsc->set_guest_state = set_guest_state;
>>> +    cgsc->set_guest_policy = set_guest_policy;
>>>    cgsc->set_id_block = set_id_block;
>>>    cgsc->get_mem_map_entry = get_mem_map_entry;
>>> }
>>> diff --git a/backends/igvm.c b/backends/igvm.c
>>> index 6545382546..5131ee7829 100644
>>> --- a/backends/igvm.c
>>> +++ b/backends/igvm.c
>>> @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
>>> 
>>>    if (guest->compatibility_mask & ctx->compatibility_mask) {
>>>        ctx->sev_policy = guest->policy;
>>> +        if (ctx->cgsc) {
>>> +            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
>>> +                                               guest->policy, errp);
>>> +        }
>>>    }
>>>    return 0;
>>> }
>>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>>> index c76cdba8d2..533ea4b54e 100644
>>> --- a/target/i386/sev.c
>>> +++ b/target/i386/sev.c
>>> @@ -128,6 +128,8 @@ struct SevCommonState {
>>>    bool kernel_hashes;
>>>    uint64_t sev_features;
>>>    uint64_t supported_sev_features;
>>> +    /* whether the guest policy was explicitly set on the command line */
>>> +    bool policy_set;
>>> 
>>>    /* runtime state */
>>>    uint8_t api_major;
>>> @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>>>    return 0;
>>> }
>>> 
>>> +static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>> +                                uint64_t policy, Error **errp)
>>> +{
>>> +    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>>> +
>>> +    if (policy_type != GUEST_POLICY_SEV) {
>>> +        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
>>> +                   policy_type);
>>> +        return -1;
>>> +    }
>>> +
>>> +    if (sev_snp_enabled()) {
>>> +        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
>>> +
>>> +        if (sev_common->policy_set &&
>>> +            sev_snp_guest->kvm_start_conf.policy != policy) {
>>> +            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
>>> +            return -1;
>>> +        }
>>> +
>>> +        sev_snp_guest->kvm_start_conf.policy = policy;
> 
> Thanks for you review!
> 
>> 
>> I had fixed a bug initially here where we need to check if the policy passed is not 0. I am not sure if that fix is still needed here.
>> Please test this scenario:
>> a) Generate an IGVM file with policy set to 0.
> 
> 0 is not a valid sev-snp guest policy: bit 17 is reserved and must be 1.
> It's a valid sev/sev-es policy though.
> 
>> b) Start a confidential SEV-SNP guest with policy set in command line and with this IGVM.
>> I think with your patch this will fail as the policies won’t match.
> 
> Correct: this was suggested by Gerd, as this is something unexpected. I
> think it would break launch measurement.

Yes I think the right thing to do is that if the policy is correctly set by IGVM, override the one set in the cli with the one in IGVM. Otherwise ignore IGVM policy.

> 
>> This potentially breaks some tests. For example, with your patch, does my FUKI confidential tests still pass?
> 
> To be fair, guest policy set via igvm _never_ worked. Only the policies
> set from CLI worked.
> 
> That said, I'll try to run your tests.
> 
> Luigi

Re: [PATCH 1/4] sev: rename set_guest_policy to set_id_block and remove dead policy code

[Stefano Garzarella]

On Tue, Sep 01, 2026 at 12:09:18PM +0200, Luigi Leonardi wrote:
>The guest policy must be provided at guest launch start: LAUNCH_START for
>SEV/SEV-ES and SNP_LAUNCH_START for SEV-SNP. See the SEV API
>specification, chapter 3 (Guest Policy), and the SEV-SNP firmware ABI
>specification, section 4.3 (Guest Policy).
>
>The policy parameter in set_guest_policy was never effective: by the
>time this callback runs, LAUNCH_START has already been issued for both
>SEV/SEV-ES and SEV-SNP, so writing to kvm_start_conf.policy or
>sev_guest->policy has no effect. In practice the only thing this
>callback actually does is set the ID block and ID auth for SNP's
>LAUNCH_FINISH, so rename it to set_id_block to reflect its real
>purpose, remove the unused policy parameter, and drop the non-SNP code
>path which was entirely dead.
>
>This is preliminary work: actually forwarding the guest policy to the
>platform before LAUNCH_START is added in a later commit.

IIUC the behaviour is the same after this patch, but IMO better to
clarify.

>
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> backends/confidential-guest-support.c       |  12 ++-
> backends/igvm.c                             |   6 +-
> include/system/confidential-guest-support.h |  28 ++++---
> target/i386/sev.c                           | 118 +++++++++++-----------------
> 4 files changed, 67 insertions(+), 97 deletions(-)
>
>diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>index 156dd15e66..a0b36d2da5 100644
>--- a/backends/confidential-guest-support.c
>+++ b/backends/confidential-guest-support.c
>@@ -38,14 +38,12 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>     return -1;
> }
>
>-static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>-                            uint64_t policy,
>-                            void *policy_data1, uint32_t policy_data1_size,
>-                            void *policy_data2, uint32_t policy_data2_size,
>-                            Error **errp)
>+static int set_id_block(void *id_block, uint32_t id_block_size,
>+                        void *id_auth, uint32_t id_auth_size,
>+                        Error **errp)
> {
>     error_setg(errp,
>-               "Setting confidential guest policy is not supported for this platform");
>+               "Setting ID block is not supported for this platform");
>     return -1;
> }
>
>@@ -64,7 +62,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>     ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>     cgsc->check_support = check_support;
>     cgsc->set_guest_state = set_guest_state;
>-    cgsc->set_guest_policy = set_guest_policy;
>+    cgsc->set_id_block = set_id_block;
>     cgsc->get_mem_map_entry = get_mem_map_entry;
> }
>
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 7b7bdc72b7..85de0d54ec 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -968,9 +968,9 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
>             id_block_len = sizeof(struct sev_id_block);
>             id_auth_len = sizeof(struct sev_id_authentication);
>         }
>-        return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV, 
>ctx->sev_policy,
>-                                          ctx->id_block, id_block_len,
>-                                          ctx->id_auth, id_auth_len, errp);
>+
>+        return ctx->cgsc->set_id_block(ctx->id_block, id_block_len,
>+                                       ctx->id_auth, id_auth_len, errp);
>     }
>     return 0;
> }
>diff --git a/include/system/confidential-guest-support.h b/include/system/confidential-guest-support.h
>index 5dca717308..6d35ddb97a 100644
>--- a/include/system/confidential-guest-support.h
>+++ b/include/system/confidential-guest-support.h
>@@ -128,21 +128,23 @@ typedef struct ConfidentialGuestSupportClass {
>                            uint16_t cpu_index, Error **errp);
>
>     /*
>-     * Set the guest policy. The policy can be used to configure the
>-     * confidential platform, such as if debug is enabled or not and can contain
>-     * information about expected launch measurements, signed verification of
>-     * guest configuration and other platform data.
>-     *
>-     * The format of the policy data is specific to each platform. For example,
>-     * SEV-SNP uses a policy bitfield in the 'policy' argument and provides an
>-     * ID block and ID authentication in the 'policy_data' parameters. The type
>-     * of policy data is identified by the 'policy_type' argument.
>+     * Set the guest policy for the confidential platform. The policy
>+     * configures properties of the guest, such as whether debug is
>+     * enabled. Its format is platform-specific; for SEV/SEV-ES and
>+     * SEV-SNP it is a policy bitfield. Must be called before LAUNCH_START
>+     * so the policy is in effect for launch.
>      */
>     int (*set_guest_policy)(ConfidentialGuestPolicyType policy_type,

I'm confused, this commit says "rename set_guest_policy to set_id_block"
so why this callback is still here?

>-                            uint64_t policy,
>-                            void *policy_data1, uint32_t policy_data1_size,
>-                            void *policy_data2, uint32_t policy_data2_size,
>-                            Error **errp);
>+                            uint64_t policy, Error **errp);
>+
>+    /*
>+     * Set the SEV-SNP ID block and ID authentication block. These are
>+     * passed to SNP_LAUNCH_FINISH to provide signed verification of the
>+     * guest configuration.
>+     */
>+    int (*set_id_block)(void *id_block, uint32_t id_block_size,
>+                        void *id_auth, uint32_t id_auth_size,
>+                        Error **errp);
>
>     /*
>      * Iterate the system memory map, getting the entry with the given index
>diff --git a/target/i386/sev.c b/target/i386/sev.c
>index 4d875d10ff..465415c535 100644
>--- a/target/i386/sev.c
>+++ b/target/i386/sev.c
>@@ -2723,10 +2723,9 @@ static int cgs_get_mem_map_entry(int index,
>     return 0;
> }
>
>-static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>-                                uint64_t policy, void *policy_data1,
>-                                uint32_t policy_data1_size, void *policy_data2,
>-                                uint32_t policy_data2_size, Error **errp)
>+static int cgs_set_id_block(void *id_block, uint32_t id_block_size,
>+                            void *id_auth, uint32_t id_auth_size,
>+                            Error **errp)
> {
>     SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>     if (sev_common->state == SEV_STATE_UNINIT) {
>@@ -2734,86 +2733,57 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>         return 0;
>     }
>
>-    if (policy_type != GUEST_POLICY_SEV) {
>-        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
>-                   policy_type);
>+    if (!sev_snp_enabled()) {
>+        error_setg(errp, "SEV: ID block is only supported for SEV-SNP");
>         return -1;
>     }
>-    /*
>-     * SEV-SNP handles policy differently. The policy flags are defined in
>-     * kvm_start_conf.policy and an ID block and ID auth can be provided.
>-     */
>-    if (sev_snp_enabled()) {
>-        SevSnpGuestState *sev_snp_guest =
>-            SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
>-        struct kvm_sev_snp_launch_finish *finish =
>-            &sev_snp_guest->kvm_finish_conf;
>
>-        /*
>-         * The policy consists of flags in 'policy' and optionally an ID block
>-         * and ID auth in policy_data1 and policy_data2 respectively. The ID
>-         * block and auth are optional so clear any previous ID block and auth
>-         * and set them if provided, but always set the policy flags.
>-         */
>-        g_free(sev_snp_guest->id_block);
>-        g_free((guchar *)finish->id_block_uaddr);
>-        g_free(sev_snp_guest->id_auth);
>-        g_free((guchar *)finish->id_auth_uaddr);
>-        sev_snp_guest->id_block = NULL;
>-        finish->id_block_uaddr = 0;
>-        sev_snp_guest->id_auth = NULL;
>-        finish->id_auth_uaddr = 0;
>-
>-        if (policy_data1_size > 0) {
>-            struct sev_snp_id_authentication *id_auth =
>-                (struct sev_snp_id_authentication *)policy_data2;
>-
>-            if (policy_data1_size != KVM_SEV_SNP_ID_BLOCK_SIZE) {
>-                error_setg(errp, "SEV: Invalid SEV-SNP ID block: incorrect size");
>-                return -1;
>-            }
>-            if (policy_data2_size != KVM_SEV_SNP_ID_AUTH_SIZE) {
>-                error_setg(errp,
>-                           "SEV: Invalid SEV-SNP ID auth block: incorrect size");
>-                return -1;
>-            }
>-            assert(policy_data1 != NULL);
>-            assert(policy_data2 != NULL);
>+    SevSnpGuestState *sev_snp_guest =
>+        SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
>+    struct kvm_sev_snp_launch_finish *finish =
>+        &sev_snp_guest->kvm_finish_conf;
>
>-            finish->id_block_uaddr =
>-                (__u64)g_memdup2(policy_data1, KVM_SEV_SNP_ID_BLOCK_SIZE);
>-            finish->id_auth_uaddr =
>-                (__u64)g_memdup2(policy_data2, KVM_SEV_SNP_ID_AUTH_SIZE);
>+    g_free(sev_snp_guest->id_block);
>+    g_free((guchar *)finish->id_block_uaddr);
>+    g_free(sev_snp_guest->id_auth);
>+    g_free((guchar *)finish->id_auth_uaddr);
>+    sev_snp_guest->id_block = NULL;
>+    finish->id_block_uaddr = 0;
>+    sev_snp_guest->id_auth = NULL;
>+    finish->id_auth_uaddr = 0;
>
>-            /*
>-             * Check if an author key has been provided and use that to flag
>-             * whether the author key is enabled. The first of the author key
>-             * must be non-zero to indicate the key type, which will currently
>-             * always be 2.
>-             */
>-            sev_snp_guest->kvm_finish_conf.auth_key_en =
>-                id_auth->author_key[0] ? 1 : 0;
>-            finish->id_block_en = 1;
>-        }
>+    if (id_block_size > 0) {
>+        struct sev_snp_id_authentication *auth =
>+            (struct sev_snp_id_authentication *)id_auth;
>
>-        /* do not reset existing policy if policy was not set in IGVM  */
>-        if (policy != 0) {
>-            sev_snp_guest->kvm_start_conf.policy = policy;
>+        if (id_block_size != KVM_SEV_SNP_ID_BLOCK_SIZE) {
>+            error_setg(errp, "SEV: Invalid SEV-SNP ID block: incorrect size");
>+            return -1;
>         }
>-    } else {
>-        SevGuestState *sev_guest = SEV_GUEST(MACHINE(qdev_get_machine())->cgs);
>-        /* Only the policy flags are supported for SEV and SEV-ES */
>-        if ((policy_data1_size > 0) || (policy_data2_size > 0) || !sev_guest) {
>-            error_setg(errp, "SEV: An ID block/ID auth block has been provided "
>-                             "but SEV-SNP is not enabled");
>+        if (id_auth_size != KVM_SEV_SNP_ID_AUTH_SIZE) {
>+            error_setg(errp,
>+                       "SEV: Invalid SEV-SNP ID auth block: incorrect size");
>             return -1;
>         }
>+        assert(id_block != NULL);
>+        assert(id_auth != NULL);
>
>-        /* do not reset existing policy if policy was not set in IGVM  */
>-        if (policy != 0) {
>-            sev_guest->policy = policy;
>-        }
>+        finish->id_block_uaddr =
>+            (__u64)g_memdup2(id_block, KVM_SEV_SNP_ID_BLOCK_SIZE);
>+        finish->id_auth_uaddr =
>+            (__u64)g_memdup2(id_auth, KVM_SEV_SNP_ID_AUTH_SIZE);
>+
>+        /*
>+         * Check if an author key has been provided and use that to flag
>+         * whether the author key is enabled. The first of the author key
>+         * must be non-zero to indicate the key type, which will currently
>+         * always be 2.
>+         */
>+        sev_snp_guest->kvm_finish_conf.auth_key_en =
>+            auth->author_key[0] ? 1 : 0;
>+        finish->id_block_en = 1;
>     }
>+
>     return 0;
> }
>
>@@ -2878,7 +2848,7 @@ sev_common_instance_init(Object *obj)
>     cgs->check_support = cgs_check_support;
>     cgs->set_guest_state = cgs_set_guest_state;
>     cgs->get_mem_map_entry = cgs_get_mem_map_entry;
>-    cgs->set_guest_policy = cgs_set_guest_policy;
>+    cgs->set_id_block = cgs_set_id_block;
>     cgs->can_rebuild_guest_state = true;
>
>     QTAILQ_INIT(&sev_common->launch_vmsa);
>
>-- 
>2.55.0
>

Re: [PATCH 1/4] sev: rename set_guest_policy to set_id_block and remove dead policy code

[Luigi Leonardi]

On Thu, Sep 03, 2026 at 12:32:21PM +0200, Stefano Garzarella wrote:
>On Tue, Sep 01, 2026 at 12:09:18PM +0200, Luigi Leonardi wrote:
>>The guest policy must be provided at guest launch start: LAUNCH_START for
>>SEV/SEV-ES and SNP_LAUNCH_START for SEV-SNP. See the SEV API
>>specification, chapter 3 (Guest Policy), and the SEV-SNP firmware ABI
>>specification, section 4.3 (Guest Policy).
>>
>>The policy parameter in set_guest_policy was never effective: by the
>>time this callback runs, LAUNCH_START has already been issued for both
>>SEV/SEV-ES and SEV-SNP, so writing to kvm_start_conf.policy or
>>sev_guest->policy has no effect. In practice the only thing this
>>callback actually does is set the ID block and ID auth for SNP's
>>LAUNCH_FINISH, so rename it to set_id_block to reflect its real
>>purpose, remove the unused policy parameter, and drop the non-SNP code
>>path which was entirely dead.
>>
>>This is preliminary work: actually forwarding the guest policy to the
>>platform before LAUNCH_START is added in a later commit.
>
>IIUC the behaviour is the same after this patch, but IMO better to
>clarify.

Yep, will do.

>
>>
>>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>>---
>>backends/confidential-guest-support.c       |  12 ++-
>>backends/igvm.c                             |   6 +-
>>include/system/confidential-guest-support.h |  28 ++++---
>>target/i386/sev.c                           | 118 +++++++++++-----------------
>>4 files changed, 67 insertions(+), 97 deletions(-)
>>
>>diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>>index 156dd15e66..a0b36d2da5 100644
>>--- a/backends/confidential-guest-support.c
>>+++ b/backends/confidential-guest-support.c
>>@@ -38,14 +38,12 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>>    return -1;
>>}
>>
>>-static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>-                            uint64_t policy,
>>-                            void *policy_data1, uint32_t policy_data1_size,
>>-                            void *policy_data2, uint32_t policy_data2_size,
>>-                            Error **errp)
>>+static int set_id_block(void *id_block, uint32_t id_block_size,
>>+                        void *id_auth, uint32_t id_auth_size,
>>+                        Error **errp)
>>{
>>    error_setg(errp,
>>-               "Setting confidential guest policy is not supported for this platform");
>>+               "Setting ID block is not supported for this platform");
>>    return -1;
>>}
>>
>>@@ -64,7 +62,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>>    ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>>    cgsc->check_support = check_support;
>>    cgsc->set_guest_state = set_guest_state;
>>-    cgsc->set_guest_policy = set_guest_policy;
>>+    cgsc->set_id_block = set_id_block;
>>    cgsc->get_mem_map_entry = get_mem_map_entry;
>>}
>>
>>diff --git a/backends/igvm.c b/backends/igvm.c
>>index 7b7bdc72b7..85de0d54ec 100644
>>--- a/backends/igvm.c
>>+++ b/backends/igvm.c
>>@@ -968,9 +968,9 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
>>            id_block_len = sizeof(struct sev_id_block);
>>            id_auth_len = sizeof(struct sev_id_authentication);
>>        }
>>-        return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV, 
>>ctx->sev_policy,
>>-                                          ctx->id_block, id_block_len,
>>-                                          ctx->id_auth, id_auth_len, errp);
>>+
>>+        return ctx->cgsc->set_id_block(ctx->id_block, id_block_len,
>>+                                       ctx->id_auth, id_auth_len, errp);
>>    }
>>    return 0;
>>}
>>diff --git a/include/system/confidential-guest-support.h b/include/system/confidential-guest-support.h
>>index 5dca717308..6d35ddb97a 100644
>>--- a/include/system/confidential-guest-support.h
>>+++ b/include/system/confidential-guest-support.h
>>@@ -128,21 +128,23 @@ typedef struct ConfidentialGuestSupportClass {
>>                           uint16_t cpu_index, Error **errp);
>>
>>    /*
>>-     * Set the guest policy. The policy can be used to configure the
>>-     * confidential platform, such as if debug is enabled or not and can contain
>>-     * information about expected launch measurements, signed verification of
>>-     * guest configuration and other platform data.
>>-     *
>>-     * The format of the policy data is specific to each platform. For example,
>>-     * SEV-SNP uses a policy bitfield in the 'policy' argument and provides an
>>-     * ID block and ID authentication in the 'policy_data' parameters. The type
>>-     * of policy data is identified by the 'policy_type' argument.
>>+     * Set the guest policy for the confidential platform. The policy
>>+     * configures properties of the guest, such as whether debug is
>>+     * enabled. Its format is platform-specific; for SEV/SEV-ES and
>>+     * SEV-SNP it is a policy bitfield. Must be called before LAUNCH_START
>>+     * so the policy is in effect for launch.
>>     */
>>    int (*set_guest_policy)(ConfidentialGuestPolicyType policy_type,
>
>I'm confused, this commit says "rename set_guest_policy to set_id_block"
>so why this callback is still here?
>

We will need this callback in a following patch, so I thought that removing
it to then add it again was pointless. If you prefer I can go this
route, or I can specify it in the commit message.

Luigi

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Luigi Leonardi]

On Thu, Sep 03, 2026 at 03:33:30PM +0530, Ani Sinha wrote:
>
>
>> On 3 Sep 2026, at 2:31 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>
>> Hi Ani,
>>
>> On Thu, Sep 03, 2026 at 02:16:30PM +0530, Ani Sinha wrote:
>>>
>>>
>>>> On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>>>
>>>> The guest policy carried in the IGVM guest-policy initialization header
>>>> was parsed into QIgvm but never forwarded to the confidential guest
>>>> platform: the previous callback ran at the end of qigvm_process_file,
>>>> after LAUNCH_START had already been issued, so writing the policy had
>>>> no effect.
>>>>
>>>> Add a set_guest_policy callback and invoke it from the guest-policy
>>>> initialization handler, so the policy reaches the platform before
>>>> LAUNCH_START.
>>>>
>>>> The guest policy can also be set on the command line. As it is part of
>>>> the attestation report, silently overriding it would cause attestation
>>>> to fail, so return an error if the command-line value differs from the
>>>> one supplied by the IGVM file.
>>>>
>>>> Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
>>>> Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
>>>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>>>> ---
>>>> backends/confidential-guest-support.c |  9 ++++++++
>>>> backends/igvm.c                       |  4 ++++
>>>> target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 52 insertions(+)
>>>>
>>>> diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>>>> index a0b36d2da5..c0d15b4a76 100644
>>>> --- a/backends/confidential-guest-support.c
>>>> +++ b/backends/confidential-guest-support.c
>>>> @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>>>>    return -1;
>>>> }
>>>>
>>>> +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>>> +                            uint64_t policy, Error **errp)
>>>> +{
>>>> +    error_setg(errp,
>>>> +               "Setting guest policy is not supported for this platform");
>>>> +    return -1;
>>>> +}
>>>> +
>>>> static int set_id_block(void *id_block, uint32_t id_block_size,
>>>>                        void *id_auth, uint32_t id_auth_size,
>>>>                        Error **errp)
>>>> @@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>>>>    ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>>>>    cgsc->check_support = check_support;
>>>>    cgsc->set_guest_state = set_guest_state;
>>>> +    cgsc->set_guest_policy = set_guest_policy;
>>>>    cgsc->set_id_block = set_id_block;
>>>>    cgsc->get_mem_map_entry = get_mem_map_entry;
>>>> }
>>>> diff --git a/backends/igvm.c b/backends/igvm.c
>>>> index 6545382546..5131ee7829 100644
>>>> --- a/backends/igvm.c
>>>> +++ b/backends/igvm.c
>>>> @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
>>>>
>>>>    if (guest->compatibility_mask & ctx->compatibility_mask) {
>>>>        ctx->sev_policy = guest->policy;
>>>> +        if (ctx->cgsc) {
>>>> +            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
>>>> +                                               guest->policy, errp);
>>>> +        }
>>>>    }
>>>>    return 0;
>>>> }
>>>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>>>> index c76cdba8d2..533ea4b54e 100644
>>>> --- a/target/i386/sev.c
>>>> +++ b/target/i386/sev.c
>>>> @@ -128,6 +128,8 @@ struct SevCommonState {
>>>>    bool kernel_hashes;
>>>>    uint64_t sev_features;
>>>>    uint64_t supported_sev_features;
>>>> +    /* whether the guest policy was explicitly set on the command line */
>>>> +    bool policy_set;
>>>>
>>>>    /* runtime state */
>>>>    uint8_t api_major;
>>>> @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>>>>    return 0;
>>>> }
>>>>
>>>> +static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>>> +                                uint64_t policy, Error **errp)
>>>> +{
>>>> +    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>>>> +
>>>> +    if (policy_type != GUEST_POLICY_SEV) {
>>>> +        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
>>>> +                   policy_type);
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    if (sev_snp_enabled()) {
>>>> +        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
>>>> +
>>>> +        if (sev_common->policy_set &&
>>>> +            sev_snp_guest->kvm_start_conf.policy != policy) {
>>>> +            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
>>>> +            return -1;
>>>> +        }
>>>> +
>>>> +        sev_snp_guest->kvm_start_conf.policy = policy;
>>
>> Thanks for you review!
>>
>>>
>>> I had fixed a bug initially here where we need to check if the policy passed is not 0. I am not sure if that fix is still needed here.
>>> Please test this scenario:
>>> a) Generate an IGVM file with policy set to 0.
>>
>> 0 is not a valid sev-snp guest policy: bit 17 is reserved and must be 1.
>> It's a valid sev/sev-es policy though.
>>
>>> b) Start a confidential SEV-SNP guest with policy set in command line and with this IGVM.
>>> I think with your patch this will fail as the policies won’t match.
>>
>> Correct: this was suggested by Gerd, as this is something unexpected. I
>> think it would break launch measurement.
>
>Yes I think the right thing to do is that if the policy is correctly set by IGVM, override the one set in the cli with the one in IGVM. Otherwise ignore IGVM policy.
>

mmh why? I might be missing something, but for now I'm not really convinced:
if we have a policy set in IGVM we should use that one. Overriding it using the
CLI, may cause measurement failure. If we have an IGVM image with a wrong policy
set, then the problem should be fixed there.

@Gerd @Stefano WDYT?

Luigi

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Ani Sinha]

> On 3 Sep 2026, at 4:32 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> 
> On Thu, Sep 03, 2026 at 03:33:30PM +0530, Ani Sinha wrote:
>> 
>> 
>>> On 3 Sep 2026, at 2:31 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>> 
>>> Hi Ani,
>>> 
>>> On Thu, Sep 03, 2026 at 02:16:30PM +0530, Ani Sinha wrote:
>>>> 
>>>> 
>>>>> On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>>>> 
>>>>> The guest policy carried in the IGVM guest-policy initialization header
>>>>> was parsed into QIgvm but never forwarded to the confidential guest
>>>>> platform: the previous callback ran at the end of qigvm_process_file,
>>>>> after LAUNCH_START had already been issued, so writing the policy had
>>>>> no effect.
>>>>> 
>>>>> Add a set_guest_policy callback and invoke it from the guest-policy
>>>>> initialization handler, so the policy reaches the platform before
>>>>> LAUNCH_START.
>>>>> 
>>>>> The guest policy can also be set on the command line. As it is part of
>>>>> the attestation report, silently overriding it would cause attestation
>>>>> to fail, so return an error if the command-line value differs from the
>>>>> one supplied by the IGVM file.
>>>>> 
>>>>> Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
>>>>> Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
>>>>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>>>>> ---
>>>>> backends/confidential-guest-support.c |  9 ++++++++
>>>>> backends/igvm.c                       |  4 ++++
>>>>> target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
>>>>> 3 files changed, 52 insertions(+)
>>>>> 
>>>>> diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>>>>> index a0b36d2da5..c0d15b4a76 100644
>>>>> --- a/backends/confidential-guest-support.c
>>>>> +++ b/backends/confidential-guest-support.c
>>>>> @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>>>>>   return -1;
>>>>> }
>>>>> 
>>>>> +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>>>> +                            uint64_t policy, Error **errp)
>>>>> +{
>>>>> +    error_setg(errp,
>>>>> +               "Setting guest policy is not supported for this platform");
>>>>> +    return -1;
>>>>> +}
>>>>> +
>>>>> static int set_id_block(void *id_block, uint32_t id_block_size,
>>>>>                       void *id_auth, uint32_t id_auth_size,
>>>>>                       Error **errp)
>>>>> @@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>>>>>   ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>>>>>   cgsc->check_support = check_support;
>>>>>   cgsc->set_guest_state = set_guest_state;
>>>>> +    cgsc->set_guest_policy = set_guest_policy;
>>>>>   cgsc->set_id_block = set_id_block;
>>>>>   cgsc->get_mem_map_entry = get_mem_map_entry;
>>>>> }
>>>>> diff --git a/backends/igvm.c b/backends/igvm.c
>>>>> index 6545382546..5131ee7829 100644
>>>>> --- a/backends/igvm.c
>>>>> +++ b/backends/igvm.c
>>>>> @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
>>>>> 
>>>>>   if (guest->compatibility_mask & ctx->compatibility_mask) {
>>>>>       ctx->sev_policy = guest->policy;
>>>>> +        if (ctx->cgsc) {
>>>>> +            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
>>>>> +                                               guest->policy, errp);
>>>>> +        }
>>>>>   }
>>>>>   return 0;
>>>>> }
>>>>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>>>>> index c76cdba8d2..533ea4b54e 100644
>>>>> --- a/target/i386/sev.c
>>>>> +++ b/target/i386/sev.c
>>>>> @@ -128,6 +128,8 @@ struct SevCommonState {
>>>>>   bool kernel_hashes;
>>>>>   uint64_t sev_features;
>>>>>   uint64_t supported_sev_features;
>>>>> +    /* whether the guest policy was explicitly set on the command line */
>>>>> +    bool policy_set;
>>>>> 
>>>>>   /* runtime state */
>>>>>   uint8_t api_major;
>>>>> @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>>>>>   return 0;
>>>>> }
>>>>> 
>>>>> +static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>>>> +                                uint64_t policy, Error **errp)
>>>>> +{
>>>>> +    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>>>>> +
>>>>> +    if (policy_type != GUEST_POLICY_SEV) {
>>>>> +        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
>>>>> +                   policy_type);
>>>>> +        return -1;
>>>>> +    }
>>>>> +
>>>>> +    if (sev_snp_enabled()) {
>>>>> +        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
>>>>> +
>>>>> +        if (sev_common->policy_set &&
>>>>> +            sev_snp_guest->kvm_start_conf.policy != policy) {
>>>>> +            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
>>>>> +            return -1;
>>>>> +        }
>>>>> +
>>>>> +        sev_snp_guest->kvm_start_conf.policy = policy;
>>> 
>>> Thanks for you review!
>>> 
>>>> 
>>>> I had fixed a bug initially here where we need to check if the policy passed is not 0. I am not sure if that fix is still needed here.
>>>> Please test this scenario:
>>>> a) Generate an IGVM file with policy set to 0.
>>> 
>>> 0 is not a valid sev-snp guest policy: bit 17 is reserved and must be 1.
>>> It's a valid sev/sev-es policy though.
>>> 
>>>> b) Start a confidential SEV-SNP guest with policy set in command line and with this IGVM.
>>>> I think with your patch this will fail as the policies won’t match.
>>> 
>>> Correct: this was suggested by Gerd, as this is something unexpected. I
>>> think it would break launch measurement.
>> 
>> Yes I think the right thing to do is that if the policy is correctly set by IGVM, override the one set in the cli with the one in IGVM. Otherwise ignore IGVM policy.
>> 
> 
> mmh why? I might be missing something, but for now I'm not really convinced:
> if we have a policy set in IGVM we should use that one.

That is what I said. “Override the one set in cli with the one in IGVM”. The exception is that if the policy in igvm is wrong (say set to 0 for sev-snp case). In that case, ignore igvm one.

> Overriding it using the
> CLI, may cause measurement failure. If we have an IGVM image with a wrong policy
> set, then the problem should be fixed there.
> 
> @Gerd @Stefano WDYT?
> 
> Luigi

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Daniel P. Berrangé]

On Thu, Sep 03, 2026 at 01:02:17PM +0200, Luigi Leonardi wrote:
> On Thu, Sep 03, 2026 at 03:33:30PM +0530, Ani Sinha wrote:
> > 
> > 
> > > On 3 Sep 2026, at 2:31 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> > > 
> > > Hi Ani,
> > > 
> > > On Thu, Sep 03, 2026 at 02:16:30PM +0530, Ani Sinha wrote:
> > > > 
> > > > 
> > > > > On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
> > > > > 
> > > > > The guest policy carried in the IGVM guest-policy initialization header
> > > > > was parsed into QIgvm but never forwarded to the confidential guest
> > > > > platform: the previous callback ran at the end of qigvm_process_file,
> > > > > after LAUNCH_START had already been issued, so writing the policy had
> > > > > no effect.
> > > > > 
> > > > > Add a set_guest_policy callback and invoke it from the guest-policy
> > > > > initialization handler, so the policy reaches the platform before
> > > > > LAUNCH_START.
> > > > > 
> > > > > The guest policy can also be set on the command line. As it is part of
> > > > > the attestation report, silently overriding it would cause attestation
> > > > > to fail, so return an error if the command-line value differs from the
> > > > > one supplied by the IGVM file.
> > > > > 
> > > > > Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
> > > > > Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
> > > > > Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> > > > > ---
> > > > > backends/confidential-guest-support.c |  9 ++++++++
> > > > > backends/igvm.c                       |  4 ++++
> > > > > target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
> > > > > 3 files changed, 52 insertions(+)
> > > > > 
> > > > > diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
> > > > > index a0b36d2da5..c0d15b4a76 100644
> > > > > --- a/backends/confidential-guest-support.c
> > > > > +++ b/backends/confidential-guest-support.c
> > > > > @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
> > > > >    return -1;
> > > > > }
> > > > > 
> > > > > +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
> > > > > +                            uint64_t policy, Error **errp)
> > > > > +{
> > > > > +    error_setg(errp,
> > > > > +               "Setting guest policy is not supported for this platform");
> > > > > +    return -1;
> > > > > +}
> > > > > +
> > > > > static int set_id_block(void *id_block, uint32_t id_block_size,
> > > > >                        void *id_auth, uint32_t id_auth_size,
> > > > >                        Error **errp)
> > > > > @@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
> > > > >    ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
> > > > >    cgsc->check_support = check_support;
> > > > >    cgsc->set_guest_state = set_guest_state;
> > > > > +    cgsc->set_guest_policy = set_guest_policy;
> > > > >    cgsc->set_id_block = set_id_block;
> > > > >    cgsc->get_mem_map_entry = get_mem_map_entry;
> > > > > }
> > > > > diff --git a/backends/igvm.c b/backends/igvm.c
> > > > > index 6545382546..5131ee7829 100644
> > > > > --- a/backends/igvm.c
> > > > > +++ b/backends/igvm.c
> > > > > @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
> > > > > 
> > > > >    if (guest->compatibility_mask & ctx->compatibility_mask) {
> > > > >        ctx->sev_policy = guest->policy;
> > > > > +        if (ctx->cgsc) {
> > > > > +            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
> > > > > +                                               guest->policy, errp);
> > > > > +        }
> > > > >    }
> > > > >    return 0;
> > > > > }
> > > > > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > > > > index c76cdba8d2..533ea4b54e 100644
> > > > > --- a/target/i386/sev.c
> > > > > +++ b/target/i386/sev.c
> > > > > @@ -128,6 +128,8 @@ struct SevCommonState {
> > > > >    bool kernel_hashes;
> > > > >    uint64_t sev_features;
> > > > >    uint64_t supported_sev_features;
> > > > > +    /* whether the guest policy was explicitly set on the command line */
> > > > > +    bool policy_set;
> > > > > 
> > > > >    /* runtime state */
> > > > >    uint8_t api_major;
> > > > > @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
> > > > >    return 0;
> > > > > }
> > > > > 
> > > > > +static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
> > > > > +                                uint64_t policy, Error **errp)
> > > > > +{
> > > > > +    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> > > > > +
> > > > > +    if (policy_type != GUEST_POLICY_SEV) {
> > > > > +        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
> > > > > +                   policy_type);
> > > > > +        return -1;
> > > > > +    }
> > > > > +
> > > > > +    if (sev_snp_enabled()) {
> > > > > +        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
> > > > > +
> > > > > +        if (sev_common->policy_set &&
> > > > > +            sev_snp_guest->kvm_start_conf.policy != policy) {
> > > > > +            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
> > > > > +            return -1;
> > > > > +        }
> > > > > +
> > > > > +        sev_snp_guest->kvm_start_conf.policy = policy;
> > > 
> > > Thanks for you review!
> > > 
> > > > 
> > > > I had fixed a bug initially here where we need to check if the policy passed is not 0. I am not sure if that fix is still needed here.
> > > > Please test this scenario:
> > > > a) Generate an IGVM file with policy set to 0.
> > > 
> > > 0 is not a valid sev-snp guest policy: bit 17 is reserved and must be 1.
> > > It's a valid sev/sev-es policy though.
> > > 
> > > > b) Start a confidential SEV-SNP guest with policy set in command line and with this IGVM.
> > > > I think with your patch this will fail as the policies won’t match.
> > > 
> > > Correct: this was suggested by Gerd, as this is something unexpected. I
> > > think it would break launch measurement.
> > 
> > Yes I think the right thing to do is that if the policy is correctly set by IGVM, override the one set in the cli with the one in IGVM. Otherwise ignore IGVM policy.

As a general rule, if the user passes a parameter to QEMU, it should
always either be honoured, or result in an error. Selectively ignoring
user input has proved to be a generally bad idea, as it tends to mask
user configuration mistakes.

> mmh why? I might be missing something, but for now I'm not really convinced:
> if we have a policy set in IGVM we should use that one. Overriding it using the
> CLI, may cause measurement failure. If we have an IGVM image with a wrong policy
> set, then the problem should be fixed there.

If the user choose to override policy on the CLI, isn't it now just
their problem to also figure out what the new expected measurement
will be ? 

Why wouldn't we just honour the IGVM by default, and if the CLI
has further customizations let them override the IGVM, and leave
the user to figure out the implications.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

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

[Stefano Garzarella]

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
>

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Stefano Garzarella]

On Tue, Sep 01, 2026 at 12:09:21PM +0200, Luigi Leonardi wrote:
>The guest policy carried in the IGVM guest-policy initialization header
>was parsed into QIgvm but never forwarded to the confidential guest
>platform: the previous callback ran at the end of qigvm_process_file,
>after LAUNCH_START had already been issued, so writing the policy had
>no effect.
>
>Add a set_guest_policy callback and invoke it from the guest-policy
>initialization handler, so the policy reaches the platform before
>LAUNCH_START.
>
>The guest policy can also be set on the command line. As it is part of
>the attestation report, silently overriding it would cause attestation
>to fail, so return an error if the command-line value differs from the
>one supplied by the IGVM file.
>
>Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
>Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> backends/confidential-guest-support.c |  9 ++++++++
> backends/igvm.c                       |  4 ++++
> target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+)
>
>diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>index a0b36d2da5..c0d15b4a76 100644
>--- a/backends/confidential-guest-support.c
>+++ b/backends/confidential-guest-support.c
>@@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>     return -1;
> }
>
>+static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>+                            uint64_t policy, Error **errp)
>+{
>+    error_setg(errp,
>+               "Setting guest policy is not supported for this platform");
>+    return -1;
>+}
>+
> static int set_id_block(void *id_block, uint32_t id_block_size,
>                         void *id_auth, uint32_t id_auth_size,
>                         Error **errp)
>@@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>     ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>     cgsc->check_support = check_support;
>     cgsc->set_guest_state = set_guest_state;
>+    cgsc->set_guest_policy = set_guest_policy;
>     cgsc->set_id_block = set_id_block;
>     cgsc->get_mem_map_entry = get_mem_map_entry;
> }
>diff --git a/backends/igvm.c b/backends/igvm.c
>index 6545382546..5131ee7829 100644
>--- a/backends/igvm.c
>+++ b/backends/igvm.c
>@@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
>
>     if (guest->compatibility_mask & ctx->compatibility_mask) {
>         ctx->sev_policy = guest->policy;
>+        if (ctx->cgsc) {
>+            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
>+                                               guest->policy, errp);
>+        }
>     }
>     return 0;
> }
>diff --git a/target/i386/sev.c b/target/i386/sev.c
>index c76cdba8d2..533ea4b54e 100644
>--- a/target/i386/sev.c
>+++ b/target/i386/sev.c
>@@ -128,6 +128,8 @@ struct SevCommonState {
>     bool kernel_hashes;
>     uint64_t sev_features;
>     uint64_t supported_sev_features;
>+    /* whether the guest policy was explicitly set on the command line */
>+    bool policy_set;
>
>     /* runtime state */
>     uint8_t api_major;
>@@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>     return 0;
> }
>
>+static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>+                                uint64_t policy, Error **errp)
>+{
>+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>+
>+    if (policy_type != GUEST_POLICY_SEV) {
>+        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
>+                   policy_type);
>+        return -1;
>+    }
>+
>+    if (sev_snp_enabled()) {
>+        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
>+
>+        if (sev_common->policy_set &&
>+            sev_snp_guest->kvm_start_conf.policy != policy) {
>+            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
>+            return -1;
>+        }
>+
>+        sev_snp_guest->kvm_start_conf.policy = policy;
>+    } else {
>+        SevGuestState *sev_guest = SEV_GUEST(sev_common);
>+
>+        if (sev_common->policy_set && sev_guest->policy != policy) {
>+            error_setg(errp, "SEV: policy mismatch between IGVM and CLI");
>+            return -1;
>+        }
>+
>+        sev_guest->policy = policy;

`sev_guest->policy` is u32, should we add a check before this 
assignment?

Stefano

>+    }
>+    return 0;
>+}
>+
> static int cgs_set_id_block(void *id_block, uint32_t id_block_size,
>                             void *id_auth, uint32_t id_auth_size,
>                             Error **errp)
>@@ -2848,6 +2884,7 @@ sev_common_instance_init(Object *obj)
>     cgs->check_support = cgs_check_support;
>     cgs->set_guest_state = cgs_set_guest_state;
>     cgs->get_mem_map_entry = cgs_get_mem_map_entry;
>+    cgs->set_guest_policy = cgs_set_guest_policy;
>     cgs->set_id_block = cgs_set_id_block;
>     cgs->can_rebuild_guest_state = true;
>
>@@ -2970,6 +3007,7 @@ sev_guest_set_policy(Object *obj, Visitor *v, const char *name,
>     if (!visit_type_uint32(v, name, &SEV_GUEST(obj)->policy, errp)) {
>         return;
>     }
>+    SEV_COMMON(obj)->policy_set = true;
> }
>
> static void
>@@ -3027,6 +3065,7 @@ sev_snp_guest_set_policy(Object *obj, Visitor *v, const char *name,
>                            errp)) {
>         return;
>     }
>+    SEV_COMMON(obj)->policy_set = true;
> }
>
> static char *
>
>-- 
>2.55.0
>

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Stefano Garzarella]

On Thu, Sep 03, 2026 at 12:50:31PM +0100, Daniel P. Berrangé wrote:
>On Thu, Sep 03, 2026 at 01:02:17PM +0200, Luigi Leonardi wrote:
>> On Thu, Sep 03, 2026 at 03:33:30PM +0530, Ani Sinha wrote:
>> >
>> >
>> > > On 3 Sep 2026, at 2:31 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>> > >
>> > > Hi Ani,
>> > >
>> > > On Thu, Sep 03, 2026 at 02:16:30PM +0530, Ani Sinha wrote:
>> > > >
>> > > >
>> > > > > On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>> > > > >
>> > > > > The guest policy carried in the IGVM guest-policy initialization header
>> > > > > was parsed into QIgvm but never forwarded to the confidential guest
>> > > > > platform: the previous callback ran at the end of qigvm_process_file,
>> > > > > after LAUNCH_START had already been issued, so writing the policy had
>> > > > > no effect.
>> > > > >
>> > > > > Add a set_guest_policy callback and invoke it from the guest-policy
>> > > > > initialization handler, so the policy reaches the platform before
>> > > > > LAUNCH_START.
>> > > > >
>> > > > > The guest policy can also be set on the command line. As it is part of
>> > > > > the attestation report, silently overriding it would cause attestation
>> > > > > to fail, so return an error if the command-line value differs from the
>> > > > > one supplied by the IGVM file.
>> > > > >
>> > > > > Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
>> > > > > Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
>> > > > > Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>> > > > > ---
>> > > > > backends/confidential-guest-support.c |  9 ++++++++
>> > > > > backends/igvm.c                       |  4 ++++
>> > > > > target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
>> > > > > 3 files changed, 52 insertions(+)
>> > > > >
>> > > > > diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>> > > > > index a0b36d2da5..c0d15b4a76 100644
>> > > > > --- a/backends/confidential-guest-support.c
>> > > > > +++ b/backends/confidential-guest-support.c
>> > > > > @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>> > > > >    return -1;
>> > > > > }
>> > > > >
>> > > > > +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>> > > > > +                            uint64_t policy, Error **errp)
>> > > > > +{
>> > > > > +    error_setg(errp,
>> > > > > +               "Setting guest policy is not supported for this platform");
>> > > > > +    return -1;
>> > > > > +}
>> > > > > +
>> > > > > static int set_id_block(void *id_block, uint32_t id_block_size,
>> > > > >                        void *id_auth, uint32_t id_auth_size,
>> > > > >                        Error **errp)
>> > > > > @@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>> > > > >    ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>> > > > >    cgsc->check_support = check_support;
>> > > > >    cgsc->set_guest_state = set_guest_state;
>> > > > > +    cgsc->set_guest_policy = set_guest_policy;
>> > > > >    cgsc->set_id_block = set_id_block;
>> > > > >    cgsc->get_mem_map_entry = get_mem_map_entry;
>> > > > > }
>> > > > > diff --git a/backends/igvm.c b/backends/igvm.c
>> > > > > index 6545382546..5131ee7829 100644
>> > > > > --- a/backends/igvm.c
>> > > > > +++ b/backends/igvm.c
>> > > > > @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
>> > > > >
>> > > > >    if (guest->compatibility_mask & ctx->compatibility_mask) {
>> > > > >        ctx->sev_policy = guest->policy;
>> > > > > +        if (ctx->cgsc) {
>> > > > > +            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
>> > > > > +                                               guest->policy, errp);
>> > > > > +        }
>> > > > >    }
>> > > > >    return 0;
>> > > > > }
>> > > > > diff --git a/target/i386/sev.c b/target/i386/sev.c
>> > > > > index c76cdba8d2..533ea4b54e 100644
>> > > > > --- a/target/i386/sev.c
>> > > > > +++ b/target/i386/sev.c
>> > > > > @@ -128,6 +128,8 @@ struct SevCommonState {
>> > > > >    bool kernel_hashes;
>> > > > >    uint64_t sev_features;
>> > > > >    uint64_t supported_sev_features;
>> > > > > +    /* whether the guest policy was explicitly set on the command line */
>> > > > > +    bool policy_set;
>> > > > >
>> > > > >    /* runtime state */
>> > > > >    uint8_t api_major;
>> > > > > @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>> > > > >    return 0;
>> > > > > }
>> > > > >
>> > > > > +static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>> > > > > +                                uint64_t policy, Error **errp)
>> > > > > +{
>> > > > > +    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>> > > > > +
>> > > > > +    if (policy_type != GUEST_POLICY_SEV) {
>> > > > > +        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
>> > > > > +                   policy_type);
>> > > > > +        return -1;
>> > > > > +    }
>> > > > > +
>> > > > > +    if (sev_snp_enabled()) {
>> > > > > +        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
>> > > > > +
>> > > > > +        if (sev_common->policy_set &&
>> > > > > +            sev_snp_guest->kvm_start_conf.policy != policy) {
>> > > > > +            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
>> > > > > +            return -1;
>> > > > > +        }
>> > > > > +
>> > > > > +        sev_snp_guest->kvm_start_conf.policy = policy;
>> > >
>> > > Thanks for you review!
>> > >
>> > > >
>> > > > I had fixed a bug initially here where we need to check if the policy passed is not 0. I am not sure if that fix is still needed here.
>> > > > Please test this scenario:
>> > > > a) Generate an IGVM file with policy set to 0.
>> > >
>> > > 0 is not a valid sev-snp guest policy: bit 17 is reserved and must be 1.
>> > > It's a valid sev/sev-es policy though.
>> > >
>> > > > b) Start a confidential SEV-SNP guest with policy set in command line and with this IGVM.
>> > > > I think with your patch this will fail as the policies won’t match.
>> > >
>> > > Correct: this was suggested by Gerd, as this is something unexpected. I
>> > > think it would break launch measurement.
>> >
>> > Yes I think the right thing to do is that if the policy is correctly set by IGVM, override the one set in the cli with the one in IGVM. Otherwise ignore IGVM policy.
>
>As a general rule, if the user passes a parameter to QEMU, it should
>always either be honoured, or result in an error. Selectively ignoring
>user input has proved to be a generally bad idea, as it tends to mask
>user configuration mistakes.
>
>> mmh why? I might be missing something, but for now I'm not really convinced:
>> if we have a policy set in IGVM we should use that one. Overriding it using the
>> CLI, may cause measurement failure. If we have an IGVM image with a wrong policy
>> set, then the problem should be fixed there.
>
>If the user choose to override policy on the CLI, isn't it now just
>their problem to also figure out what the new expected measurement
>will be ?
>
>Why wouldn't we just honour the IGVM by default, and if the CLI
>has further customizations let them override the IGVM, and leave
>the user to figure out the implications.

I also slightly prefer this behaviour too, I see some advantages, 
especially for testing and debugging, where you don't want to regenerate 
the IGVM. But I don't have a strong opinion on this; if there is a 
mismatch, though, I agree that it's better to get an error than to 
ignore the CLI.

Thanks,
Stefano

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

[Luigi Leonardi]

On Thu, Sep 03, 2026 at 02:57:44PM +0200, Stefano Garzarella wrote:
>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.

This is a very good question: id block per snp spec *requires* a policy
to be set. So can we consider an IGVM file that contains a id block directive
but not guest policy to be valid? If so, I need to modify the code and read
the policy from `kvm_start_conf` with a new callback.

Luigi

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Luigi Leonardi]

On Thu, Sep 03, 2026 at 03:28:29PM +0200, Stefano Garzarella wrote:
>On Thu, Sep 03, 2026 at 12:50:31PM +0100, Daniel P. Berrangé wrote:
>>On Thu, Sep 03, 2026 at 01:02:17PM +0200, Luigi Leonardi wrote:
>>>On Thu, Sep 03, 2026 at 03:33:30PM +0530, Ani Sinha wrote:
>>>>
>>>>
>>>> > On 3 Sep 2026, at 2:31 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>>> >
>>>> > Hi Ani,
>>>> >
>>>> > On Thu, Sep 03, 2026 at 02:16:30PM +0530, Ani Sinha wrote:
>>>> > >
>>>> > >
>>>> > > > On 1 Sep 2026, at 3:39 PM, Luigi Leonardi <leonardi@redhat.com> wrote:
>>>> > > >
>>>> > > > The guest policy carried in the IGVM guest-policy initialization header
>>>> > > > was parsed into QIgvm but never forwarded to the confidential guest
>>>> > > > platform: the previous callback ran at the end of qigvm_process_file,
>>>> > > > after LAUNCH_START had already been issued, so writing the policy had
>>>> > > > no effect.
>>>> > > >
>>>> > > > Add a set_guest_policy callback and invoke it from the guest-policy
>>>> > > > initialization handler, so the policy reaches the platform before
>>>> > > > LAUNCH_START.
>>>> > > >
>>>> > > > The guest policy can also be set on the command line. As it is part of
>>>> > > > the attestation report, silently overriding it would cause attestation
>>>> > > > to fail, so return an error if the command-line value differs from the
>>>> > > > one supplied by the IGVM file.
>>>> > > >
>>>> > > > Link: https://gitlab.com/qemu-project/qemu/-/work_items/4189
>>>> > > > Fixes: 915b47078d ("backends/igvm: Handle policy for SEV guests")
>>>> > > > Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>>>> > > > ---
>>>> > > > backends/confidential-guest-support.c |  9 ++++++++
>>>> > > > backends/igvm.c                       |  4 ++++
>>>> > > > target/i386/sev.c                     | 39 +++++++++++++++++++++++++++++++++++
>>>> > > > 3 files changed, 52 insertions(+)
>>>> > > >
>>>> > > > diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>>>> > > > index a0b36d2da5..c0d15b4a76 100644
>>>> > > > --- a/backends/confidential-guest-support.c
>>>> > > > +++ b/backends/confidential-guest-support.c
>>>> > > > @@ -38,6 +38,14 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>>>> > > >    return -1;
>>>> > > > }
>>>> > > >
>>>> > > > +static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>>> > > > +                            uint64_t policy, Error **errp)
>>>> > > > +{
>>>> > > > +    error_setg(errp,
>>>> > > > +               "Setting guest policy is not supported for this platform");
>>>> > > > +    return -1;
>>>> > > > +}
>>>> > > > +
>>>> > > > static int set_id_block(void *id_block, uint32_t id_block_size,
>>>> > > >                        void *id_auth, uint32_t id_auth_size,
>>>> > > >                        Error **errp)
>>>> > > > @@ -62,6 +70,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>>>> > > >    ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>>>> > > >    cgsc->check_support = check_support;
>>>> > > >    cgsc->set_guest_state = set_guest_state;
>>>> > > > +    cgsc->set_guest_policy = set_guest_policy;
>>>> > > >    cgsc->set_id_block = set_id_block;
>>>> > > >    cgsc->get_mem_map_entry = get_mem_map_entry;
>>>> > > > }
>>>> > > > diff --git a/backends/igvm.c b/backends/igvm.c
>>>> > > > index 6545382546..5131ee7829 100644
>>>> > > > --- a/backends/igvm.c
>>>> > > > +++ b/backends/igvm.c
>>>> > > > @@ -868,6 +868,10 @@ static int qigvm_initialization_guest_policy(QIgvm *ctx,
>>>> > > >
>>>> > > >    if (guest->compatibility_mask & ctx->compatibility_mask) {
>>>> > > >        ctx->sev_policy = guest->policy;
>>>> > > > +        if (ctx->cgsc) {
>>>> > > > +            return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
>>>> > > > +                                               guest->policy, errp);
>>>> > > > +        }
>>>> > > >    }
>>>> > > >    return 0;
>>>> > > > }
>>>> > > > diff --git a/target/i386/sev.c b/target/i386/sev.c
>>>> > > > index c76cdba8d2..533ea4b54e 100644
>>>> > > > --- a/target/i386/sev.c
>>>> > > > +++ b/target/i386/sev.c
>>>> > > > @@ -128,6 +128,8 @@ struct SevCommonState {
>>>> > > >    bool kernel_hashes;
>>>> > > >    uint64_t sev_features;
>>>> > > >    uint64_t supported_sev_features;
>>>> > > > +    /* whether the guest policy was explicitly set on the command line */
>>>> > > > +    bool policy_set;
>>>> > > >
>>>> > > >    /* runtime state */
>>>> > > >    uint8_t api_major;
>>>> > > > @@ -2723,6 +2725,40 @@ static int cgs_get_mem_map_entry(int index,
>>>> > > >    return 0;
>>>> > > > }
>>>> > > >
>>>> > > > +static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>>> > > > +                                uint64_t policy, Error **errp)
>>>> > > > +{
>>>> > > > +    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
>>>> > > > +
>>>> > > > +    if (policy_type != GUEST_POLICY_SEV) {
>>>> > > > +        error_setg(errp, "SEV: Invalid guest policy type provided for SEV: %d",
>>>> > > > +                   policy_type);
>>>> > > > +        return -1;
>>>> > > > +    }
>>>> > > > +
>>>> > > > +    if (sev_snp_enabled()) {
>>>> > > > +        SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(sev_common);
>>>> > > > +
>>>> > > > +        if (sev_common->policy_set &&
>>>> > > > +            sev_snp_guest->kvm_start_conf.policy != policy) {
>>>> > > > +            error_setg(errp, "SNP: policy mismatch between IGVM and CLI");
>>>> > > > +            return -1;
>>>> > > > +        }
>>>> > > > +
>>>> > > > +        sev_snp_guest->kvm_start_conf.policy = policy;
>>>> >
>>>> > Thanks for you review!
>>>> >
>>>> > >
>>>> > > I had fixed a bug initially here where we need to check if the policy passed is not 0. I am not sure if that fix is still needed here.
>>>> > > Please test this scenario:
>>>> > > a) Generate an IGVM file with policy set to 0.
>>>> >
>>>> > 0 is not a valid sev-snp guest policy: bit 17 is reserved and must be 1.
>>>> > It's a valid sev/sev-es policy though.
>>>> >
>>>> > > b) Start a confidential SEV-SNP guest with policy set in command line and with this IGVM.
>>>> > > I think with your patch this will fail as the policies won’t match.
>>>> >
>>>> > Correct: this was suggested by Gerd, as this is something unexpected. I
>>>> > think it would break launch measurement.
>>>>
>>>> Yes I think the right thing to do is that if the policy is correctly set by IGVM, override the one set in the cli with the one in IGVM. Otherwise ignore IGVM policy.
>>
>>As a general rule, if the user passes a parameter to QEMU, it should
>>always either be honoured, or result in an error. Selectively ignoring
>>user input has proved to be a generally bad idea, as it tends to mask
>>user configuration mistakes.
>>
>>>mmh why? I might be missing something, but for now I'm not really convinced:
>>>if we have a policy set in IGVM we should use that one. Overriding it using the
>>>CLI, may cause measurement failure. If we have an IGVM image with a wrong policy
>>>set, then the problem should be fixed there.
>>
>>If the user choose to override policy on the CLI, isn't it now just
>>their problem to also figure out what the new expected measurement
>>will be ?
>>
>>Why wouldn't we just honour the IGVM by default, and if the CLI
>>has further customizations let them override the IGVM, and leave
>>the user to figure out the implications.
>
>I also slightly prefer this behaviour too, I see some advantages, 
>especially for testing and debugging, where you don't want to 
>regenerate the IGVM. But I don't have a strong opinion on this; if 
>there is a mismatch, though, I agree that it's better to get an error 
>than to ignore the CLI.
>

In this series, if there is a mismatch between IGVM and CLI, QEMU returns an error.
Debugging sounds very reasonable to me, so I'm fine on letting the CLI override IGVM.

Luigi

Re: [PATCH 1/4] sev: rename set_guest_policy to set_id_block and remove dead policy code

[Stefano Garzarella]

On Thu, Sep 03, 2026 at 12:47:00PM +0200, Luigi Leonardi wrote:
>On Thu, Sep 03, 2026 at 12:32:21PM +0200, Stefano Garzarella wrote:
>>On Tue, Sep 01, 2026 at 12:09:18PM +0200, Luigi Leonardi wrote:
>>>The guest policy must be provided at guest launch start: LAUNCH_START for
>>>SEV/SEV-ES and SNP_LAUNCH_START for SEV-SNP. See the SEV API
>>>specification, chapter 3 (Guest Policy), and the SEV-SNP firmware ABI
>>>specification, section 4.3 (Guest Policy).
>>>
>>>The policy parameter in set_guest_policy was never effective: by the
>>>time this callback runs, LAUNCH_START has already been issued for both
>>>SEV/SEV-ES and SEV-SNP, so writing to kvm_start_conf.policy or
>>>sev_guest->policy has no effect. In practice the only thing this
>>>callback actually does is set the ID block and ID auth for SNP's
>>>LAUNCH_FINISH, so rename it to set_id_block to reflect its real
>>>purpose, remove the unused policy parameter, and drop the non-SNP code
>>>path which was entirely dead.
>>>
>>>This is preliminary work: actually forwarding the guest policy to the
>>>platform before LAUNCH_START is added in a later commit.
>>
>>IIUC the behaviour is the same after this patch, but IMO better to
>>clarify.
>
>Yep, will do.
>
>>
>>>
>>>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>>>---
>>>backends/confidential-guest-support.c       |  12 ++-
>>>backends/igvm.c                             |   6 +-
>>>include/system/confidential-guest-support.h |  28 ++++---
>>>target/i386/sev.c                           | 118 +++++++++++-----------------
>>>4 files changed, 67 insertions(+), 97 deletions(-)
>>>
>>>diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
>>>index 156dd15e66..a0b36d2da5 100644
>>>--- a/backends/confidential-guest-support.c
>>>+++ b/backends/confidential-guest-support.c
>>>@@ -38,14 +38,12 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
>>>   return -1;
>>>}
>>>
>>>-static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
>>>-                            uint64_t policy,
>>>-                            void *policy_data1, uint32_t policy_data1_size,
>>>-                            void *policy_data2, uint32_t policy_data2_size,
>>>-                            Error **errp)
>>>+static int set_id_block(void *id_block, uint32_t id_block_size,
>>>+                        void *id_auth, uint32_t id_auth_size,
>>>+                        Error **errp)
>>>{
>>>   error_setg(errp,
>>>-               "Setting confidential guest policy is not supported for this platform");
>>>+               "Setting ID block is not supported for this platform");
>>>   return -1;
>>>}
>>>
>>>@@ -64,7 +62,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
>>>   ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
>>>   cgsc->check_support = check_support;
>>>   cgsc->set_guest_state = set_guest_state;
>>>-    cgsc->set_guest_policy = set_guest_policy;
>>>+    cgsc->set_id_block = set_id_block;
>>>   cgsc->get_mem_map_entry = get_mem_map_entry;
>>>}
>>>
>>>diff --git a/backends/igvm.c b/backends/igvm.c
>>>index 7b7bdc72b7..85de0d54ec 100644
>>>--- a/backends/igvm.c
>>>+++ b/backends/igvm.c
>>>@@ -968,9 +968,9 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
>>>           id_block_len = sizeof(struct sev_id_block);
>>>           id_auth_len = sizeof(struct sev_id_authentication);
>>>       }
>>>-        return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV, 
>>>ctx->sev_policy,
>>>-                                          ctx->id_block, id_block_len,
>>>-                                          ctx->id_auth, id_auth_len, errp);
>>>+
>>>+        return ctx->cgsc->set_id_block(ctx->id_block, id_block_len,
>>>+                                       ctx->id_auth, id_auth_len, errp);
>>>   }
>>>   return 0;
>>>}
>>>diff --git a/include/system/confidential-guest-support.h b/include/system/confidential-guest-support.h
>>>index 5dca717308..6d35ddb97a 100644
>>>--- a/include/system/confidential-guest-support.h
>>>+++ b/include/system/confidential-guest-support.h
>>>@@ -128,21 +128,23 @@ typedef struct ConfidentialGuestSupportClass {
>>>                          uint16_t cpu_index, Error **errp);
>>>
>>>   /*
>>>-     * Set the guest policy. The policy can be used to configure the
>>>-     * confidential platform, such as if debug is enabled or not and can contain
>>>-     * information about expected launch measurements, signed verification of
>>>-     * guest configuration and other platform data.
>>>-     *
>>>-     * The format of the policy data is specific to each platform. For example,
>>>-     * SEV-SNP uses a policy bitfield in the 'policy' argument and provides an
>>>-     * ID block and ID authentication in the 'policy_data' parameters. The type
>>>-     * of policy data is identified by the 'policy_type' argument.
>>>+     * Set the guest policy for the confidential platform. The policy
>>>+     * configures properties of the guest, such as whether debug is
>>>+     * enabled. Its format is platform-specific; for SEV/SEV-ES and
>>>+     * SEV-SNP it is a policy bitfield. Must be called before LAUNCH_START
>>>+     * so the policy is in effect for launch.
>>>    */
>>>   int (*set_guest_policy)(ConfidentialGuestPolicyType policy_type,
>>
>>I'm confused, this commit says "rename set_guest_policy to set_id_block"
>>so why this callback is still here?
>>
>
>We will need this callback in a following patch, so I thought that removing
>it to then add it again was pointless. If you prefer I can go this
>route, or I can specify it in the commit message.

I see your point about avoiding remove and re-add the callback, but
self-contained commits aren't a matter of preference or taste, they're
a maintainability requirement. A callback declared but unimplemented
across three commits leaves the API in an incomplete state.

Please move the set_guest_policy declaration where it's first used.

Thanks,
Stefano

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

[Stefano Garzarella]

On Thu, Sep 03, 2026 at 03:29:35PM +0200, Luigi Leonardi wrote:
>On Thu, Sep 03, 2026 at 02:57:44PM +0200, Stefano Garzarella wrote:
>>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.
>
>This is a very good question: id block per snp spec *requires* a policy
>to be set. So can we consider an IGVM file that contains a id block directive
>but not guest policy to be valid? If so, I need to modify the code and read
>the policy from `kvm_start_conf` with a new callback.

Yes, maybe we could throw an error in this case; otherwise, the only 
other option I see is to use the same default we use for the CLI, and at 
this point, reading `kvm_start_conf` might help you keep the default in 
just one place.

Stefano

Re: [PATCH 1/4] sev: rename set_guest_policy to set_id_block and remove dead policy code

[Gerd Hoffmann]

On Thu, Sep 03, 2026 at 03:55:31PM +0200, Stefano Garzarella wrote:
> On Thu, Sep 03, 2026 at 12:47:00PM +0200, Luigi Leonardi wrote:
> > On Thu, Sep 03, 2026 at 12:32:21PM +0200, Stefano Garzarella wrote:
> > > On Tue, Sep 01, 2026 at 12:09:18PM +0200, Luigi Leonardi wrote:
> > > > The guest policy must be provided at guest launch start: LAUNCH_START for
> > > > SEV/SEV-ES and SNP_LAUNCH_START for SEV-SNP. See the SEV API
> > > > specification, chapter 3 (Guest Policy), and the SEV-SNP firmware ABI
> > > > specification, section 4.3 (Guest Policy).
> > > > 
> > > > The policy parameter in set_guest_policy was never effective: by the
> > > > time this callback runs, LAUNCH_START has already been issued for both
> > > > SEV/SEV-ES and SEV-SNP, so writing to kvm_start_conf.policy or
> > > > sev_guest->policy has no effect. In practice the only thing this
> > > > callback actually does is set the ID block and ID auth for SNP's
> > > > LAUNCH_FINISH, so rename it to set_id_block to reflect its real
> > > > purpose, remove the unused policy parameter, and drop the non-SNP code
> > > > path which was entirely dead.
> > > > 
> > > > This is preliminary work: actually forwarding the guest policy to the
> > > > platform before LAUNCH_START is added in a later commit.
> > > 
> > > IIUC the behaviour is the same after this patch, but IMO better to
> > > clarify.
> > 
> > Yep, will do.
> > 
> > > 
> > > > 
> > > > Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> > > > ---
> > > > backends/confidential-guest-support.c       |  12 ++-
> > > > backends/igvm.c                             |   6 +-
> > > > include/system/confidential-guest-support.h |  28 ++++---
> > > > target/i386/sev.c                           | 118 +++++++++++-----------------
> > > > 4 files changed, 67 insertions(+), 97 deletions(-)
> > > > 
> > > > diff --git a/backends/confidential-guest-support.c b/backends/confidential-guest-support.c
> > > > index 156dd15e66..a0b36d2da5 100644
> > > > --- a/backends/confidential-guest-support.c
> > > > +++ b/backends/confidential-guest-support.c
> > > > @@ -38,14 +38,12 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
> > > >   return -1;
> > > > }
> > > > 
> > > > -static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
> > > > -                            uint64_t policy,
> > > > -                            void *policy_data1, uint32_t policy_data1_size,
> > > > -                            void *policy_data2, uint32_t policy_data2_size,
> > > > -                            Error **errp)
> > > > +static int set_id_block(void *id_block, uint32_t id_block_size,
> > > > +                        void *id_auth, uint32_t id_auth_size,
> > > > +                        Error **errp)
> > > > {
> > > >   error_setg(errp,
> > > > -               "Setting confidential guest policy is not supported for this platform");
> > > > +               "Setting ID block is not supported for this platform");
> > > >   return -1;
> > > > }
> > > > 
> > > > @@ -64,7 +62,7 @@ static void confidential_guest_support_class_init(ObjectClass *oc,
> > > >   ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
> > > >   cgsc->check_support = check_support;
> > > >   cgsc->set_guest_state = set_guest_state;
> > > > -    cgsc->set_guest_policy = set_guest_policy;
> > > > +    cgsc->set_id_block = set_id_block;
> > > >   cgsc->get_mem_map_entry = get_mem_map_entry;
> > > > }
> > > > 
> > > > diff --git a/backends/igvm.c b/backends/igvm.c
> > > > index 7b7bdc72b7..85de0d54ec 100644
> > > > --- a/backends/igvm.c
> > > > +++ b/backends/igvm.c
> > > > @@ -968,9 +968,9 @@ static int qigvm_handle_policy(QIgvm *ctx, Error **errp)
> > > >           id_block_len = sizeof(struct sev_id_block);
> > > >           id_auth_len = sizeof(struct sev_id_authentication);
> > > >       }
> > > > -        return ctx->cgsc->set_guest_policy(GUEST_POLICY_SEV,
> > > > ctx->sev_policy,
> > > > -                                          ctx->id_block, id_block_len,
> > > > -                                          ctx->id_auth, id_auth_len, errp);
> > > > +
> > > > +        return ctx->cgsc->set_id_block(ctx->id_block, id_block_len,
> > > > +                                       ctx->id_auth, id_auth_len, errp);
> > > >   }
> > > >   return 0;
> > > > }
> > > > diff --git a/include/system/confidential-guest-support.h b/include/system/confidential-guest-support.h
> > > > index 5dca717308..6d35ddb97a 100644
> > > > --- a/include/system/confidential-guest-support.h
> > > > +++ b/include/system/confidential-guest-support.h
> > > > @@ -128,21 +128,23 @@ typedef struct ConfidentialGuestSupportClass {
> > > >                          uint16_t cpu_index, Error **errp);
> > > > 
> > > >   /*
> > > > -     * Set the guest policy. The policy can be used to configure the
> > > > -     * confidential platform, such as if debug is enabled or not and can contain
> > > > -     * information about expected launch measurements, signed verification of
> > > > -     * guest configuration and other platform data.
> > > > -     *
> > > > -     * The format of the policy data is specific to each platform. For example,
> > > > -     * SEV-SNP uses a policy bitfield in the 'policy' argument and provides an
> > > > -     * ID block and ID authentication in the 'policy_data' parameters. The type
> > > > -     * of policy data is identified by the 'policy_type' argument.
> > > > +     * Set the guest policy for the confidential platform. The policy
> > > > +     * configures properties of the guest, such as whether debug is
> > > > +     * enabled. Its format is platform-specific; for SEV/SEV-ES and
> > > > +     * SEV-SNP it is a policy bitfield. Must be called before LAUNCH_START
> > > > +     * so the policy is in effect for launch.
> > > >    */
> > > >   int (*set_guest_policy)(ConfidentialGuestPolicyType policy_type,
> > > 
> > > I'm confused, this commit says "rename set_guest_policy to set_id_block"
> > > so why this callback is still here?
> > > 
> > 
> > We will need this callback in a following patch, so I thought that removing
> > it to then add it again was pointless. If you prefer I can go this
> > route, or I can specify it in the commit message.
> 
> I see your point about avoiding remove and re-add the callback, but
> self-contained commits aren't a matter of preference or taste, they're
> a maintainability requirement. A callback declared but unimplemented
> across three commits leaves the API in an incomplete state.
> 
> Please move the set_guest_policy declaration where it's first used.

A typical and good way to handle cases like this (a fix needs some code
reorganization) is to split that into two patches:  First patch does
the code reorganization without functional change (in this case: split
one callback into two), then have a relatively small patch with the
actual bugfix (move one of the two callback calls to another place).

take care,
  Gerd

Re: [PATCH 4/4] igvm/sev: forward the IGVM guest policy to the platform before launch

[Gerd Hoffmann]

  Hi,

> > > If the user choose to override policy on the CLI, isn't it now just
> > > their problem to also figure out what the new expected measurement
> > > will be ?
> > > 
> > > Why wouldn't we just honour the IGVM by default, and if the CLI
> > > has further customizations let them override the IGVM, and leave
> > > the user to figure out the implications.
> > 
> > I also slightly prefer this behaviour too, I see some advantages,
> > especially for testing and debugging, where you don't want to regenerate
> > the IGVM. But I don't have a strong opinion on this; if there is a
> > mismatch, though, I agree that it's better to get an error than to
> > ignore the CLI.
> 
> In this series, if there is a mismatch between IGVM and CLI, QEMU returns an error.
> Debugging sounds very reasonable to me, so I'm fine on letting the CLI override IGVM.

qemu should flag this mismatch in any case.  Either throw an error and
exit, or log a warning and continue.  Given that changing the policy
changes the launch measurement I'd tend to prefer an error.  If people
see value and real use cases for policy overrides I'm fine with a
warning too.

Silently accepting the override is IMHO not an option.  We had enough
problems with qemu silently doing things not expected by users in the
past.

take care,
  Gerd