Re: [PATCH v7 15/16] i386/sev: Add implementation of CGS set_guest_policy()

[Roy Hopkins <roy.hopkins@randomman.co.uk>]

On Fri, 2025-06-13 at 17:41 +0530, Ani Sinha wrote:
> 
> 
> > On 27 Feb 2025, at 7:59 PM, Roy Hopkins <roy.hopkins@randomman.co.uk> wrote:
> > 
> > The new cgs_set_guest_policy() function is provided to receive the guest
> > policy flags, SNP ID block and SNP ID authentication from guest
> > configuration such as an IGVM file and apply it to the platform prior to
> > launching the guest.
> > 
> > The policy is used to populate values for the existing 'policy',
> > 'id_block' and 'id_auth' parameters. When provided, the guest policy is
> > applied and the ID block configuration is used to verify the launch
> > measurement and signatures. The guest is only successfully started if
> > the expected launch measurements match the actual measurements and the
> > signatures are valid.
> > 
> > Signed-off-by: Roy Hopkins <roy.hopkins@randomman.co.uk>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Acked-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> > target/i386/sev.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++
> > target/i386/sev.h | 12 +++++++
> > 2 files changed, 95 insertions(+)
> > 
> > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > index 31b29695bf..fa9b4bcad6 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> > @@ -2526,6 +2526,88 @@ 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)
> > +{
> > +    if (policy_type != GUEST_POLICY_SEV) {
> > +        error_setg(errp, "%s: Invalid guest policy type provided for SEV: %d",
> > +        __func__, policy_type);
> > +        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, "%s: Invalid SEV-SNP ID block: incorrect size",
> > +                           __func__);
> > +                return -1;
> > +            }
> > +            if (policy_data2_size != KVM_SEV_SNP_ID_AUTH_SIZE) {
> > +                error_setg(errp,
> > +                           "%s: Invalid SEV-SNP ID auth block: incorrect size",
> > +                           __func__);
> > +                return -1;
> > +            }
> > +            assert(policy_data1 != NULL);
> > +            assert(policy_data2 != NULL);
> > +
> > +            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);
> > +
> > +            /*
> > +             * 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;
> > +        }
> > +        sev_snp_guest->kvm_start_conf.policy = policy;
> > +    } else {
> 
> I do not see how this “else” part (sev and sev-es) will ever be executed since qigvm_handle_policy() in patch #14 only calls set_guest_policy() if its SEV-
> SNP.
> 
You're correct. There is currently no way for this code to be be called. But as it is
an interface function on confidential-guest-support, it could be called erroneously
in the future so I don't see any harm in leaving the error check there.

Thanks,
Roy