| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Ybtfon70/+lG63BP@zn.tnic>
Date: Thu, 16 Dec 2021 16:47:46 +0100
From: Borislav Petkov <bp@...en8.de>
To: Brijesh Singh <brijesh.singh@....com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
linux-efi@...r.kernel.org, platform-driver-x86@...r.kernel.org,
linux-coco@...ts.linux.dev, linux-mm@...ck.org,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Joerg Roedel <jroedel@...e.de>,
Tom Lendacky <thomas.lendacky@....com>,
"H. Peter Anvin" <hpa@...or.com>, Ard Biesheuvel <ardb@...nel.org>,
Paolo Bonzini <pbonzini@...hat.com>,
Sean Christopherson <seanjc@...gle.com>,
Vitaly Kuznetsov <vkuznets@...hat.com>,
Jim Mattson <jmattson@...gle.com>,
Andy Lutomirski <luto@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Sergio Lopez <slp@...hat.com>, Peter Gonda <pgonda@...gle.com>,
Peter Zijlstra <peterz@...radead.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
David Rientjes <rientjes@...gle.com>,
Dov Murik <dovmurik@...ux.ibm.com>,
Tobin Feldman-Fitzthum <tobin@....com>,
Michael Roth <michael.roth@....com>,
Vlastimil Babka <vbabka@...e.cz>,
"Kirill A . Shutemov" <kirill@...temov.name>,
Andi Kleen <ak@...ux.intel.com>,
"Dr . David Alan Gilbert" <dgilbert@...hat.com>,
tony.luck@...el.com, marcorr@...gle.com,
sathyanarayanan.kuppuswamy@...ux.intel.com
Subject: Re: [PATCH v8 06/40] x86/sev: Check SEV-SNP features support
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']
> handler to sev_enable() so that all feature detection happens before
> the first VC exception.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@....com>
> ---
> 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?
> 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);
...
> 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
> +
> +
> /* #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
Powered by blists - more mailing lists