diff mbox series

[RFC,3/6] i386/sev: initialize SNP context

Message ID 20210709215550.32496-4-brijesh.singh@amd.com (mailing list archive)
State New, archived
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) support | expand

Commit Message

Brijesh Singh July 9, 2021, 9:55 p.m. UTC 
When SEV-SNP is enabled, the KVM_SNP_INIT command is used to initialize
the platform. The command checks whether SNP is enabled in the KVM, if
enabled then it allocate a new ASID from the SNP pool and calls the
firmware to initialize the all the resources.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 target/i386/sev.c      | 24 +++++++++++++++++++++---
 target/i386/sev_i386.h |  1 +
 2 files changed, 22 insertions(+), 3 deletions(-)

Comments

Dov Murik July 15, 2021, 9:32 a.m. UTC | #1 
Hi Brijesh,

On 10/07/2021 0:55, Brijesh Singh wrote:
> When SEV-SNP is enabled, the KVM_SNP_INIT command is used to initialize
> the platform. The command checks whether SNP is enabled in the KVM, if
> enabled then it allocate a new ASID from the SNP pool and calls the

s/allocate/allocates/

> firmware to initialize the all the resources.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  target/i386/sev.c      | 24 +++++++++++++++++++++---
>  target/i386/sev_i386.h |  1 +
>  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 6b238ef969..84ae244af0 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -583,10 +583,17 @@ sev_enabled(void)
>      return !!sev_guest;
>  }
>  
> +bool
> +sev_snp_enabled(void)
> +{
> +    return sev_guest->snp;
> +}
> +
>  bool
>  sev_es_enabled(void)
>  {
> -    return sev_enabled() && (sev_guest->policy & SEV_POLICY_ES);
> +    return sev_snp_enabled() ||
> +           (sev_enabled() && (sev_guest->policy & SEV_POLICY_ES));
>  }
>  

Just making sure I understand:

* sev_enabled() returns true for SEV or newer (SEV or SEV-ES or
  SEV-SNP).
* sev_es_enabled() returns true for SEV-ES or newer (SEV-ES or SEV-SNP).
* sev_snp_enabled() returns true for SEV-SNP or newer (currently only
  SEV-SNP).

Is that indeed the intention?


-Dov


>  uint64_t
> @@ -1008,6 +1015,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>      uint32_t ebx;
>      uint32_t host_cbitpos;
>      struct sev_user_data_status status = {};
> +    void *init_args = NULL;
>  
>      if (!sev) {
>          return 0;
> @@ -1061,7 +1069,17 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>      sev->api_major = status.api_major;
>      sev->api_minor = status.api_minor;
>  
> -    if (sev_es_enabled()) {
> +    if (sev_snp_enabled()) {
> +        if (!kvm_kernel_irqchip_allowed()) {
> +            error_report("%s: SEV-SNP guests require in-kernel irqchip support",
> +                         __func__);
> +            goto err;
> +        }
> +
> +        cmd = KVM_SEV_SNP_INIT;
> +        init_args = (void *)&sev->snp_config.init;
> +
> +    } else if (sev_es_enabled()) {
>          if (!kvm_kernel_irqchip_allowed()) {
>              error_report("%s: SEV-ES guests require in-kernel irqchip support",
>                           __func__);
> @@ -1080,7 +1098,7 @@ int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>      }
>  
>      trace_kvm_sev_init();
> -    ret = sev_ioctl(sev->sev_fd, cmd, NULL, &fw_error);
> +    ret = sev_ioctl(sev->sev_fd, cmd, init_args, &fw_error);
>      if (ret) {
>          error_setg(errp, "%s: failed to initialize ret=%d fw_error=%d '%s'",
>                     __func__, ret, fw_error, fw_error_to_str(fw_error));
> diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
> index ae6d840478..e0e1a599be 100644
> --- a/target/i386/sev_i386.h
> +++ b/target/i386/sev_i386.h
> @@ -29,6 +29,7 @@
>  #define SEV_POLICY_SEV          0x20
>  
>  extern bool sev_es_enabled(void);
> +extern bool sev_snp_enabled(void);
>  extern uint64_t sev_get_me_mask(void);
>  extern SevInfo *sev_get_info(void);
>  extern uint32_t sev_get_cbit_position(void);
>
Brijesh Singh July 15, 2021, 1:24 p.m. UTC | #2 
On 7/15/21 4:32 AM, Dov Murik wrote:
> 
> Just making sure I understand:
> 
> * sev_enabled() returns true for SEV or newer (SEV or SEV-ES or
>    SEV-SNP).
> * sev_es_enabled() returns true for SEV-ES or newer (SEV-ES or SEV-SNP).
> * sev_snp_enabled() returns true for SEV-SNP or newer (currently only
>    SEV-SNP).
> 
> Is that indeed the intention?
> 

Yes. The SEV-SNP support requires the SEV and SEV-ES to be enabled. See 
the text from the APM vol2 section 15.36.

	The SEV-SNP features enable additional protection for encrypted
	VMs designed to achieve stronger isolation from the hypervisor.
	SEV-SNP is used with the SEV and SEV-ES features described in
	Section 15.34 and Section 15.35 respectively and requires the
	enablement and use of these features.

thanks
diff mbox series

Patch

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 6b238ef969..84ae244af0 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -583,10 +583,17 @@  sev_enabled(void)
     return !!sev_guest;
 }
 
+bool
+sev_snp_enabled(void)
+{
+    return sev_guest->snp;
+}
+
 bool
 sev_es_enabled(void)
 {
-    return sev_enabled() && (sev_guest->policy & SEV_POLICY_ES);
+    return sev_snp_enabled() ||
+           (sev_enabled() && (sev_guest->policy & SEV_POLICY_ES));
 }
 
 uint64_t
@@ -1008,6 +1015,7 @@  int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
     uint32_t ebx;
     uint32_t host_cbitpos;
     struct sev_user_data_status status = {};
+    void *init_args = NULL;
 
     if (!sev) {
         return 0;
@@ -1061,7 +1069,17 @@  int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
     sev->api_major = status.api_major;
     sev->api_minor = status.api_minor;
 
-    if (sev_es_enabled()) {
+    if (sev_snp_enabled()) {
+        if (!kvm_kernel_irqchip_allowed()) {
+            error_report("%s: SEV-SNP guests require in-kernel irqchip support",
+                         __func__);
+            goto err;
+        }
+
+        cmd = KVM_SEV_SNP_INIT;
+        init_args = (void *)&sev->snp_config.init;
+
+    } else if (sev_es_enabled()) {
         if (!kvm_kernel_irqchip_allowed()) {
             error_report("%s: SEV-ES guests require in-kernel irqchip support",
                          __func__);
@@ -1080,7 +1098,7 @@  int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
     }
 
     trace_kvm_sev_init();
-    ret = sev_ioctl(sev->sev_fd, cmd, NULL, &fw_error);
+    ret = sev_ioctl(sev->sev_fd, cmd, init_args, &fw_error);
     if (ret) {
         error_setg(errp, "%s: failed to initialize ret=%d fw_error=%d '%s'",
                    __func__, ret, fw_error, fw_error_to_str(fw_error));
diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index ae6d840478..e0e1a599be 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -29,6 +29,7 @@ 
 #define SEV_POLICY_SEV          0x20
 
 extern bool sev_es_enabled(void);
+extern bool sev_snp_enabled(void);
 extern uint64_t sev_get_me_mask(void);
 extern SevInfo *sev_get_info(void);
 extern uint32_t sev_get_cbit_position(void);