On Fri, Jun 21, 2024 at 03:29:18PM +0100, Roy Hopkins wrote:
> IGVM files can contain an initial VMSA that should be applied to each
> vcpu as part of the initial guest state. The sev_features flags are
> provided as part of the VMSA structure. However, KVM only allows
> sev_features to be set during initialization and not as the guest is
> being prepared for launch.
>
> This patch queries KVM for the supported set of sev_features flags and
> processes the IGVM file during kvm_init to determine any sev_features
> flags set in the IGVM file. These are then provided in the call to
> KVM_SEV_INIT2 to ensure the guest state matches that specified in the
> IGVM file.
>
> This does cause the IGVM file to be processed twice. Firstly to extract
> the sev_features then secondly to actually configure the guest. However,
> the first pass is largely ignored meaning the overhead is minimal.
>
> Signed-off-by: Roy Hopkins <roy.hopk...@suse.com>
> ---
> target/i386/sev.c | 145 ++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 126 insertions(+), 19 deletions(-)
>
> static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> {
> char *devname;
> @@ -1743,6 +1783,10 @@ static int
> sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> }
> }
>
> + if (sev_init_supported_features(sev_common, errp) < 0) {
> + return -1;
> + }
> +
> trace_kvm_sev_init();
> if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) ==
> KVM_X86_DEFAULT_VM) {
> cmd = sev_es_enabled() ? KVM_SEV_ES_INIT : KVM_SEV_INIT;
> @@ -1750,6 +1794,38 @@ static int
> sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> ret = sev_ioctl(sev_common->sev_fd, cmd, NULL, &fw_error);
> } else {
> struct kvm_sev_init args = { 0 };
> + MachineState *machine = MACHINE(qdev_get_machine());
> +
> + /*
> + * If configuration is provided via an IGVM file then the IGVM file
> + * might contain configuration of the initial vcpu context. For SEV
> + * the vcpu context includes the sev_features which should be applied
> + * to the vcpu.
> + *
> + * KVM does not synchronize sev_features from CPU state. Instead it
> + * requires sev_features to be provided as part of this
> initialization
> + * call which is subsequently automatically applied to the VMSA of
> + * each vcpu.
> + *
> + * The IGVM file is normally processed after initialization.
> Therefore
> + * we need to pre-process it here to extract sev_features in order to
> + * provide it to KVM_SEV_INIT2. Each cgs_* function that is called by
> + * the IGVM processor detects this pre-process by observing the state
> + * as SEV_STATE_UNINIT.
> + */
> + if (machine->igvm) {
> + if (IGVM_CFG_GET_CLASS(machine->igvm)
> + ->process(machine->igvm, machine->cgs, errp) == -1) {
> + return -1;
> + }
> + /*
> + * KVM maintains a bitmask of allowed sev_features. This does not
> + * include SVM_SEV_FEAT_SNP_ACTIVE which is set accordingly by
> KVM
> + * itself. Therefore we need to clear this flag.
> + */
> + args.vmsa_features = sev_common->sev_features &
> + ~SVM_SEV_FEAT_SNP_ACTIVE;
> + }
>
> ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
> }
What happens if the code path takes us down the KVM_SEV_INIT
route, rather than KVM_SEV_INIT2 ? Should we be reporting an
error indicating that IGVM usage is incompatible with the legacy
KVM_SEV_INIT path ?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
