diff mbox series

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

Message ID 8e49a1aed06d4f98c13a98fbbe3a48cdeb6e0070.1718979106.git.roy.hopkins@suse.com (mailing list archive)
State New
Headers show
Series Introduce support for IGVM files | expand

Commit Message

Roy Hopkins June 21, 2024, 2:29 p.m. UTC
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(-)

Comments

Daniel P. Berrangé June 24, 2024, 2:14 p.m. UTC | #1
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
diff mbox series

Patch

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 688b378c42..4b9d74207b 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -117,6 +117,8 @@  struct SevCommonState {
     uint32_t cbitpos;
     uint32_t reduced_phys_bits;
     bool kernel_hashes;
+    uint64_t sev_features;
+    uint64_t supported_sev_features;
 
     /* runtime state */
     uint8_t api_major;
@@ -490,7 +492,40 @@  static void sev_apply_cpu_context(CPUState *cpu)
     }
 }
 
-static int check_vmsa_supported(hwaddr gpa, const struct sev_es_save_area *vmsa,
+static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
+                              Error **errp)
+{
+    /*
+     * Ensure SEV_FEATURES is configured for correct SEV hardware and that
+     * the requested features are supported. If SEV-SNP is enabled then
+     * that feature must be enabled, otherwise it must be cleared.
+     */
+    if (sev_snp_enabled() && !(sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
+        error_setg(
+            errp,
+            "%s: SEV_SNP is enabled but is not enabled in VMSA sev_features",
+            __func__);
+        return -1;
+    } else if (!sev_snp_enabled() &&
+               (sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
+        error_setg(
+            errp,
+            "%s: SEV_SNP is not enabled but is enabled in VMSA sev_features",
+            __func__);
+        return -1;
+    }
+    if (sev_features & ~sev_common->supported_sev_features) {
+        error_setg(errp,
+                   "%s: VMSA contains unsupported sev_features: %lX, "
+                   "supported features: %lX",
+                   __func__, sev_features, sev_common->supported_sev_features);
+        return -1;
+    }
+    return 0;
+}
+
+static int check_vmsa_supported(SevCommonState *sev_common, hwaddr gpa,
+                                const struct sev_es_save_area *vmsa,
                                 Error **errp)
 {
     struct sev_es_save_area vmsa_check;
@@ -556,24 +591,10 @@  static int check_vmsa_supported(hwaddr gpa, const struct sev_es_save_area *vmsa,
     vmsa_check.x87_fcw = 0;
     vmsa_check.mxcsr = 0;
 
-    if (sev_snp_enabled()) {
-        if (vmsa_check.sev_features != SVM_SEV_FEAT_SNP_ACTIVE) {
-            error_setg(errp,
-                       "%s: sev_features in the VMSA contains an unsupported "
-                       "value. For SEV-SNP, sev_features must be set to %x.",
-                       __func__, SVM_SEV_FEAT_SNP_ACTIVE);
-            return -1;
-        }
-        vmsa_check.sev_features = 0;
-    } else {
-        if (vmsa_check.sev_features != 0) {
-            error_setg(errp,
-                       "%s: sev_features in the VMSA contains an unsupported "
-                       "value. For SEV-ES and SEV, sev_features must be "
-                       "set to 0.", __func__);
-            return -1;
-        }
+    if (check_sev_features(sev_common, vmsa_check.sev_features, errp) < 0) {
+        return -1;
     }
+    vmsa_check.sev_features = 0;
 
     if (!buffer_is_zero(&vmsa_check, sizeof(vmsa_check))) {
         error_setg(errp,
@@ -1663,6 +1684,25 @@  static int sev_snp_kvm_type(X86ConfidentialGuest *cg)
     return KVM_X86_SNP_VM;
 }
 
+static int sev_init_supported_features(SevCommonState *sev_common, Error **errp)
+{
+    /* Query KVM for the supported set of sev_features */
+    struct kvm_device_attr attr = {
+        .group = KVM_X86_GRP_SEV,
+        .attr = KVM_X86_SEV_VMSA_FEATURES,
+        .addr = (unsigned long)&sev_common->supported_sev_features,
+    };
+    if (kvm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr) < 0) {
+        error_setg(errp, "%s: failed to query supported sev_features",
+                   __func__);
+        return -1;
+    }
+    if (sev_snp_enabled()) {
+        sev_common->supported_sev_features |= SVM_SEV_FEAT_SNP_ACTIVE;
+    }
+    return 0;
+}
+
 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);
     }
@@ -2348,6 +2424,24 @@  static int cgs_set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
     SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
     SevCommonStateClass *klass = SEV_COMMON_GET_CLASS(sev_common);
 
+    if (sev_common->state == SEV_STATE_UNINIT) {
+        /* Pre-processing of IGVM file called from sev_common_kvm_init() */
+        if ((cpu_index == 0) && (memory_type == CGS_PAGE_TYPE_VMSA)) {
+            const struct sev_es_save_area *sa =
+                (const struct sev_es_save_area *)ptr;
+            if (len < sizeof(*sa)) {
+                error_setg(errp, "%s: invalid VMSA length encountered",
+                           __func__);
+                return -1;
+            }
+            if (check_sev_features(sev_common, sa->sev_features, errp) < 0) {
+                return -1;
+            }
+            sev_common->sev_features = sa->sev_features;
+        }
+        return 0;
+    }
+
     if (!sev_enabled()) {
         error_setg(errp, "%s: attempt to configure guest memory, but SEV "
                      "is not enabled", __func__);
@@ -2367,7 +2461,8 @@  static int cgs_set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
                        __func__);
             return -1;
         }
-        if (check_vmsa_supported(gpa, (const struct sev_es_save_area *)ptr,
+        if (check_vmsa_supported(sev_common, gpa,
+                                 (const struct sev_es_save_area *)ptr,
                                  errp) < 0) {
             return -1;
         }
@@ -2421,6 +2516,12 @@  static int cgs_get_mem_map_entry(int index,
                                  ConfidentialGuestMemoryMapEntry *entry,
                                  Error **errp)
 {
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+    if (sev_common->state == SEV_STATE_UNINIT) {
+        /* Pre-processing of IGVM file called from sev_common_kvm_init() */
+        return 1;
+    }
+
     if ((index < 0) || (index >= e820_get_num_entries())) {
         return 1;
     }
@@ -2451,6 +2552,12 @@  static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
                                 uint32_t policy_data1_size, void *policy_data2,
                                 uint32_t policy_data2_size, Error **errp)
 {
+    SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+    if (sev_common->state == SEV_STATE_UNINIT) {
+        /* Pre-processing of IGVM file called from sev_common_kvm_init() */
+        return 0;
+    }
+
     if (policy_type != GUEST_POLICY_SEV) {
         error_setg(errp, "%s: Invalid guest policy type provided for SEV: %d",
         __func__, policy_type);