lkml.org 
[lkml]   [2021]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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@amd.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

    \
     
     \ /
      Last update: 2021-12-16 16:49    [W:2.490 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site