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 <leona...@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