Re: [PATCH v8 06/40] x86/sev: Check SEV-SNP features support
From: Borislav Petkov <bp@alien8.de>
Date: 2021-12-16 15:47:53
Also in:
kvm, linux-coco, linux-efi, lkml, platform-driver-x86
On Fri, Dec 10, 2021 at 09:42:58AM -0600, Brijesh Singh wrote:
Version 2 of the GHCB specification added the advertisement of features that are supported by the hypervisor. If hypervisor supports the SEV-SNP then it must set the SEV-SNP features bit to indicate that the base SEV-SNP is supported. Check the SEV-SNP feature while establishing the GHCB, if failed, terminate the guest. Version 2 of GHCB specification adds several new NAEs, most of them are optional except the hypervisor feature. Now that hypervisor feature NAE is implemented, so bump the GHCB maximum support protocol version. While at it, move the GHCB protocol negotitation check from VC exception
Unknown word [negotitation] in commit message, suggestions:
['negotiation', 'negotiator', 'negotiate', 'abnegation', 'vegetation']
quoted hunk ↗ jump to hunk
handler to sev_enable() so that all feature detection happens before the first VC exception. Signed-off-by: Brijesh Singh <redacted> --- arch/x86/boot/compressed/sev.c | 21 ++++++++++++++++----- arch/x86/include/asm/sev-common.h | 6 ++++++ arch/x86/include/asm/sev.h | 2 +- arch/x86/include/uapi/asm/svm.h | 2 ++ arch/x86/kernel/sev-shared.c | 20 ++++++++++++++++++++ arch/x86/kernel/sev.c | 16 ++++++++++++++++ 6 files changed, 61 insertions(+), 6 deletions(-)diff --git a/arch/x86/boot/compressed/sev.c b/arch/x86/boot/compressed/sev.c index 0b6cc6402ac1..a0708f359a46 100644 --- a/arch/x86/boot/compressed/sev.c +++ b/arch/x86/boot/compressed/sev.c@@ -119,11 +119,8 @@ static enum es_result vc_read_mem(struct es_em_ctxt *ctxt, /* Include code for early handlers */ #include "../../kernel/sev-shared.c" -static bool early_setup_sev_es(void) +static bool early_setup_ghcb(void) { - if (!sev_es_negotiate_protocol()) - sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_PROT_UNSUPPORTED); - if (set_page_decrypted((unsigned long)&boot_ghcb_page)) return false;@@ -174,7 +171,7 @@ void do_boot_stage2_vc(struct pt_regs *regs, unsigned long exit_code) struct es_em_ctxt ctxt; enum es_result result; - if (!boot_ghcb && !early_setup_sev_es()) + if (!boot_ghcb && !early_setup_ghcb()) sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_GEN_REQ);
Can you setup the GHCB in sev_enable() too, after the protocol version negotiation succeeds?
quoted hunk ↗ jump to hunk
vc_ghcb_invalidate(boot_ghcb);@@ -247,5 +244,19 @@ void sev_enable(struct boot_params *bp) if (!(sev_status & MSR_AMD64_SEV_ENABLED)) return; + /* Negotiate the GHCB protocol version */ + if (sev_status & MSR_AMD64_SEV_ES_ENABLED) + if (!sev_es_negotiate_protocol()) + sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SEV_ES_PROT_UNSUPPORTED); + + /* + * SNP is supported in v2 of the GHCB spec which mandates support for HV + * features. If SEV-SNP is enabled, then check if the hypervisor supports + * the SEV-SNP features. + */ + if (sev_status & MSR_AMD64_SEV_SNP_ENABLED && !(get_hv_features() & GHCB_HV_FT_SNP)) + sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED); + +
^ Superfluous newline.
sme_me_mask = BIT_ULL(ebx & 0x3f);
...
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c index 19ad09712902..a0cada8398a4 100644 --- a/arch/x86/kernel/sev.c +++ b/arch/x86/kernel/sev.c@@ -43,6 +43,10 @@ static struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE); */ static struct ghcb __initdata *boot_ghcb; +/* Bitmap of SEV features supported by the hypervisor */ +static u64 sev_hv_features;
__ro_after_init
quoted hunk ↗ jump to hunk
+ + /* #VC handler runtime per-CPU data */ struct sev_es_runtime_data { struct ghcb ghcb_page;@@ -766,6 +770,18 @@ void __init sev_es_init_vc_handling(void) if (!sev_es_check_cpu_features()) panic("SEV-ES CPU Features missing"); + /* + * SNP is supported in v2 of the GHCB spec which mandates support for HV + * features. If SEV-SNP is enabled, then check if the hypervisor supports
s/SEV-SNP/SNP/g
And please do that everywhere in sev-specific files.
This file is called sev.c and there's way too many acronyms flying
around so the simpler the better.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette