Re: [PATCH v3 15/15] sev: Provide sev_features flags from IGVM VMSA to KVM_SEV_INIT2

["Daniel P. Berrangé" <berrange@redhat.com>]

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.hopkins@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 :|