Re: [PATCH v2 3/3] x86/sev: Add SEV-SNP CipherTextHiding support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 17, 2024 at 2:17 PM Ashish Kalra <Ashish.Kalra@xxxxxxx> wrote:
>
> From: Ashish Kalra <ashish.kalra@xxxxxxx>
>
> Ciphertext hiding prevents host accesses from reading the ciphertext of
> SNP guest private memory. Instead of reading ciphertext, the host reads
> will see constant default values (0xff).
>
> Ciphertext hiding separates the ASID space into SNP guest ASIDs and host
> ASIDs. All SNP active guests must have an ASID less than or equal to
> MAX_SNP_ASID provided to the SNP_INIT_EX command. All SEV-legacy guests
> (SEV and SEV-ES) must be greater than MAX_SNP_ASID.
>
> This patch-set adds a new module parameter to the CCP driver defined as
> max_snp_asid which is a user configurable MAX_SNP_ASID to define the
> system-wide maximum SNP ASID value. If this value is not set, then the
> ASID space is equally divided between SEV-SNP and SEV-ES guests.
>
> Ciphertext hiding needs to be enabled on SNP_INIT_EX and therefore this
> new module parameter has to added to the CCP driver.
>
> Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxx>
> ---
>  arch/x86/kvm/svm/sev.c       | 26 ++++++++++++++----
>  drivers/crypto/ccp/sev-dev.c | 52 ++++++++++++++++++++++++++++++++++++
>  include/linux/psp-sev.h      | 12 +++++++--
>  3 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 0b851ef937f2..a345b4111ad6 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -171,7 +171,7 @@ static void sev_misc_cg_uncharge(struct kvm_sev_info *sev)
>         misc_cg_uncharge(type, sev->misc_cg, 1);
>  }
>
> -static int sev_asid_new(struct kvm_sev_info *sev)
> +static int sev_asid_new(struct kvm_sev_info *sev, unsigned long vm_type)
>  {
>         /*
>          * SEV-enabled guests must use asid from min_sev_asid to max_sev_asid.
> @@ -199,6 +199,18 @@ static int sev_asid_new(struct kvm_sev_info *sev)
>
>         mutex_lock(&sev_bitmap_lock);
>
> +       /*
> +        * When CipherTextHiding is enabled, all SNP guests must have an
> +        * ASID less than or equal to MAX_SNP_ASID provided on the
> +        * SNP_INIT_EX command and all the SEV-ES guests must have
> +        * an ASID greater than MAX_SNP_ASID.
> +        */
> +       if (snp_cipher_text_hiding && sev->es_active) {
> +               if (vm_type == KVM_X86_SNP_VM)
> +                       max_asid = snp_max_snp_asid;
> +               else
> +                       min_asid = snp_max_snp_asid + 1;
> +       }
>  again:
>         asid = find_next_zero_bit(sev_asid_bitmap, max_asid + 1, min_asid);
>         if (asid > max_asid) {
> @@ -440,7 +452,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
>         if (vm_type == KVM_X86_SNP_VM)
>                 sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
>
> -       ret = sev_asid_new(sev);
> +       ret = sev_asid_new(sev, vm_type);
>         if (ret)
>                 goto e_no_asid;
>
> @@ -3059,14 +3071,18 @@ void __init sev_hardware_setup(void)
>                                                                        "unusable" :
>                                                                        "disabled",
>                         min_sev_asid, max_sev_asid);
> -       if (boot_cpu_has(X86_FEATURE_SEV_ES))
> +       if (boot_cpu_has(X86_FEATURE_SEV_ES)) {
> +               if (snp_max_snp_asid >= (min_sev_asid - 1))
> +                       sev_es_supported = false;
>                 pr_info("SEV-ES %s (ASIDs %u - %u)\n",
>                         sev_es_supported ? "enabled" : "disabled",
> -                       min_sev_asid > 1 ? 1 : 0, min_sev_asid - 1);
> +                       min_sev_asid > 1 ? snp_max_snp_asid ? snp_max_snp_asid + 1 : 1 :
> +                                                             0, min_sev_asid - 1);
> +       }
>         if (boot_cpu_has(X86_FEATURE_SEV_SNP))
>                 pr_info("SEV-SNP %s (ASIDs %u - %u)\n",
>                         sev_snp_supported ? "enabled" : "disabled",
> -                       min_sev_asid > 1 ? 1 : 0, min_sev_asid - 1);
> +                       min_sev_asid > 1 ? 1 : 0, snp_max_snp_asid ? : min_sev_asid - 1);
>
>         sev_enabled = sev_supported;
>         sev_es_enabled = sev_es_supported;
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 564daf748293..77900abb1b46 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -73,11 +73,27 @@ static bool psp_init_on_probe = true;
>  module_param(psp_init_on_probe, bool, 0444);
>  MODULE_PARM_DESC(psp_init_on_probe, "  if true, the PSP will be initialized on module init. Else the PSP will be initialized on the first command requiring it");
>
> +static bool cipher_text_hiding = true;
> +module_param(cipher_text_hiding, bool, 0444);
> +MODULE_PARM_DESC(cipher_text_hiding, "  if true, the PSP will enable Cipher Text Hiding");
> +
> +static int max_snp_asid;
> +module_param(max_snp_asid, int, 0444);
> +MODULE_PARM_DESC(max_snp_asid, "  override MAX_SNP_ASID for Cipher Text Hiding");

My read of the spec is if Ciphertext hiding is not enabled there is no
additional split in the ASID space. Am I understanding that correctly?
If so, I don't think we want to enable ciphertext hiding by default
because it might break whatever management of ASIDs systems already
have. For instance right now we have to split SEV-ES and SEV ASIDS,
and SNP guests need SEV-ES ASIDS. This change would half the # of SNP
enable ASIDs on a system.

Also should we move the ASID splitting code to be all in one place?
Right now KVM handles it in sev_hardware_setup().

> +
>  MODULE_FIRMWARE("amd/amd_sev_fam17h_model0xh.sbin"); /* 1st gen EPYC */
>  MODULE_FIRMWARE("amd/amd_sev_fam17h_model3xh.sbin"); /* 2nd gen EPYC */
>  MODULE_FIRMWARE("amd/amd_sev_fam19h_model0xh.sbin"); /* 3rd gen EPYC */
>  MODULE_FIRMWARE("amd/amd_sev_fam19h_model1xh.sbin"); /* 4th gen EPYC */
>
> +/* Cipher Text Hiding Enabled */
> +bool snp_cipher_text_hiding;
> +EXPORT_SYMBOL(snp_cipher_text_hiding);
> +
> +/* MAX_SNP_ASID */
> +unsigned int snp_max_snp_asid;
> +EXPORT_SYMBOL(snp_max_snp_asid);
> +
>  static bool psp_dead;
>  static int psp_timeout;
>
> @@ -1064,6 +1080,38 @@ static void snp_set_hsave_pa(void *arg)
>         wrmsrl(MSR_VM_HSAVE_PA, 0);
>  }
>
> +static void sev_snp_enable_ciphertext_hiding(struct sev_data_snp_init_ex *data, int *error)
> +{
> +       struct psp_device *psp = psp_master;
> +       struct sev_device *sev;
> +       unsigned int edx;
> +
> +       sev = psp->sev_data;
> +
> +       /*
> +        * Check if CipherTextHiding feature is supported and enabled
> +        * in the Platform/BIOS.
> +        */
> +       if ((sev->feat_info.ecx & SNP_CIPHER_TEXT_HIDING_SUPPORTED) &&
> +           sev->snp_plat_status.ciphertext_hiding_cap) {
> +               /* Retrieve SEV CPUID information */
> +               edx = cpuid_edx(0x8000001f);
> +               /* Do sanity checks on user-defined MAX_SNP_ASID */
> +               if (max_snp_asid >= edx) {
> +                       dev_info(sev->dev, "max_snp_asid module parameter is not valid, limiting to %d\n",
> +                                edx - 1);
> +                       max_snp_asid = edx - 1;
> +               }
> +               snp_max_snp_asid = max_snp_asid ? : (edx - 1) / 2;
> +
> +               snp_cipher_text_hiding = 1;
> +               data->ciphertext_hiding_en = 1;
> +               data->max_snp_asid = snp_max_snp_asid;
> +
> +               dev_dbg(sev->dev, "SEV-SNP CipherTextHiding feature support enabled\n");
> +       }
> +}
> +
>  static void snp_get_platform_data(void)
>  {
>         struct sev_device *sev = psp_master->sev_data;
> @@ -1199,6 +1247,10 @@ static int __sev_snp_init_locked(int *error)
>                 }
>
>                 memset(&data, 0, sizeof(data));
> +
> +               if (cipher_text_hiding)
> +                       sev_snp_enable_ciphertext_hiding(&data, error);
> +
>                 data.init_rmp = 1;
>                 data.list_paddr_en = 1;
>                 data.list_paddr = __psp_pa(snp_range_list);
> diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
> index 6068a89839e1..2102248bd436 100644
> --- a/include/linux/psp-sev.h
> +++ b/include/linux/psp-sev.h
> @@ -27,6 +27,9 @@ enum sev_state {
>         SEV_STATE_MAX
>  };
>
> +extern bool snp_cipher_text_hiding;
> +extern unsigned int snp_max_snp_asid;
> +
>  /**
>   * SEV platform and guest management commands
>   */
> @@ -746,10 +749,13 @@ struct sev_data_snp_guest_request {
>  struct sev_data_snp_init_ex {
>         u32 init_rmp:1;
>         u32 list_paddr_en:1;
> -       u32 rsvd:30;
> +       u32 rapl_dis:1;
> +       u32 ciphertext_hiding_en:1;
> +       u32 rsvd:28;
>         u32 rsvd1;
>         u64 list_paddr;
> -       u8  rsvd2[48];
> +       u16 max_snp_asid;
> +       u8  rsvd2[46];
>  } __packed;
>
>  /**
> @@ -841,6 +847,8 @@ struct snp_feature_info {
>         u32 edx;
>  } __packed;
>
> +#define SNP_CIPHER_TEXT_HIDING_SUPPORTED       BIT(3)
> +
>  #ifdef CONFIG_CRYPTO_DEV_SP_PSP
>
>  /**
> --
> 2.34.1
>
>





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux