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

[Luigi Leonardi <leonardi@redhat.com>]

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