Re: [PATCH Part1 RFC v4 04/36] x86/mm: Add sev_feature_enabled() helper

Dr. David Alan Gilbert
2021-07-08T08:50:31Z

* Brijesh Singh (brijesh.singh@amd.com) wrote:
> The sev_feature_enabled() helper can be used by the guest to query whether
> the SNP - Secure Nested Paging feature is active.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> arch/x86/include/asm/mem_encrypt.h | 8 ++++++++
> arch/x86/include/asm/msr-index.h | 2 ++
> arch/x86/mm/mem_encrypt.c | 14 ++++++++++++++
> 3 files changed, 24 insertions(+)
>
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 8cc2fd308f65..fb857f2e72cb 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -16,6 +16,12 @@
>
> #include <asm/bootparam.h>
>
> +enum sev_feature_type {
> + SEV,
> + SEV_ES,
> + SEV_SNP
> +};

Is this ....

> #ifdef CONFIG_AMD_MEM_ENCRYPT
>
> extern u64 sme_me_mask;
> @@ -54,6 +60,7 @@ void __init sev_es_init_vc_handling(void);
> bool sme_active(void);
> bool sev_active(void);
> bool sev_es_active(void);
> +bool sev_feature_enabled(unsigned int feature_type);
>
> #define __bss_decrypted __section(".bss..decrypted")
>
> @@ -87,6 +94,7 @@ static inline int __init
> early_set_memory_encrypted(unsigned long vaddr, unsigned long size) { return 0; }
>
> static inline void mem_encrypt_free_decrypted_mem(void) { }
> +static bool sev_feature_enabled(unsigned int feature_type) { return false; }
>
> #define __bss_decrypted
>
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index a7c413432b33..37589da0282e 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -481,8 +481,10 @@
> #define MSR_AMD64_SEV 0xc0010131
> #define MSR_AMD64_SEV_ENABLED_BIT 0
> #define MSR_AMD64_SEV_ES_ENABLED_BIT 1
> +#define MSR_AMD64_SEV_SNP_ENABLED_BIT 2

Just the same as this ?

> #define MSR_AMD64_SEV_ENABLED BIT_ULL(MSR_AMD64_SEV_ENABLED_BIT)
> #define MSR_AMD64_SEV_ES_ENABLED BIT_ULL(MSR_AMD64_SEV_ES_ENABLED_BIT)
> +#define MSR_AMD64_SEV_SNP_ENABLED BIT_ULL(MSR_AMD64_SEV_SNP_ENABLED_BIT)
>
> #define MSR_AMD64_VIRT_SPEC_CTRL 0xc001011f
>
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index ff08dc463634..63e7799a9a86 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -389,6 +389,16 @@ bool noinstr sev_es_active(void)
> return sev_status & MSR_AMD64_SEV_ES_ENABLED;
> }
>
> +bool sev_feature_enabled(unsigned int type)

In which case, if you want the enum then that would be enum
sev_feature_type type ?

> +{
> + switch (type) {
> + case SEV: return sev_status & MSR_AMD64_SEV_ENABLED;
> + case SEV_ES: return sev_status & MSR_AMD64_SEV_ES_ENABLED;
> + case SEV_SNP: return sev_status & MSR_AMD64_SEV_SNP_ENABLED;
> + default: return false;

or, could you just go for making that whole thing a bit test on 1<<type
?

> + }
> +}
> +
> /* Override for DMA direct allocation check - ARCH_HAS_FORCE_DMA_UNENCRYPTED */
> bool force_dma_unencrypted(struct device *dev)
> {
> @@ -461,6 +471,10 @@ static void print_mem_encrypt_feature_info(void)
> if (sev_es_active())
> pr_cont(" SEV-ES");
>
> + /* Secure Nested Paging */
> + if (sev_feature_enabled(SEV_SNP))
> + pr_cont(" SEV-SNP");
> +
> pr_cont("\n");
> }

Dave

> --
> 2.17.1
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK