2024-03-30 20:20:46

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH v12 10/29] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_START command

On 3/29/24 23:58, Michael Roth wrote:
> From: Brijesh Singh <[email protected]>
>
> KVM_SEV_SNP_LAUNCH_START begins the launch process for an SEV-SNP guest.
> The command initializes a cryptographic digest context used to construct
> the measurement of the guest. Other commands can then at that point be
> used to load/encrypt data into the guest's initial launch image.

Does KVM_SEV_LAUNCH_START fail for SNP guests, or should we take care of
forbidding it?

> + if (params.policy & SNP_POLICY_MASK_SINGLE_SOCKET) {
> + pr_debug("SEV-SNP hypervisor does not support limiting guests to a single socket.");
> + return -EINVAL;
> + }
> +
> + if (!(params.policy & SNP_POLICY_MASK_SMT)) {
> + pr_debug("SEV-SNP hypervisor does not support limiting guests to a single SMT thread.");
> + return -EINVAL;
> + }

Since you're forbidding some bits, KVM should also check that undefined
bits (63:25) are zero.

Also what about checking that the major version is equal to the one that
KVM supports? From the docs it's not even clear what ABI version they
document (QEMU uses 0).

Otherwise looks good.

Paolo