linux-kernel - Re: [RFC Part1 PATCH 08/13] x86/sev-es: register GHCB memory when SEV-SNP is active

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 8 Apr 2021 10:38:14 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Brijesh Singh <brijesh.singh@....com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, kvm@...r.kernel.org,
        ak@...ux.intel.com, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joerg Roedel <jroedel@...e.de>,
        "H. Peter Anvin" <hpa@...or.com>, Tony Luck <tony.luck@...el.com>,
        Dave Hansen <dave.hansen@...el.com>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        David Rientjes <rientjes@...gle.com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [RFC Part1 PATCH 08/13] x86/sev-es: register GHCB memory when
 SEV-SNP is active

On Wed, Mar 24, 2021 at 11:44:19AM -0500, Brijesh Singh wrote:
> @@ -88,6 +89,13 @@ struct sev_es_runtime_data {
>  	 * is currently unsupported in SEV-ES guests.
>  	 */
>  	unsigned long dr7;
> +
> +	/*
> +	 * SEV-SNP requires that the GHCB must be registered before using it.
> +	 * The flag below will indicate whether the GHCB is registered, if its
> +	 * not registered then sev_es_get_ghcb() will perform the registration.
> +	 */
> +	bool ghcb_registered;

snp_ghcb_registered

because it is SNP-specific.

>  };
>  
>  struct ghcb_state {
> @@ -196,6 +204,12 @@ static __always_inline struct ghcb *sev_es_get_ghcb(struct ghcb_state *state)
>  		data->ghcb_active = true;
>  	}
>  
> +	/* SEV-SNP guest requires that GHCB must be registered before using it. */
> +	if (sev_snp_active() && !data->ghcb_registered) {
> +		sev_snp_register_ghcb(__pa(ghcb));
> +		data->ghcb_registered = true;

This needs to be set to true in the function itself, in the success
case.

> +static inline u64 sev_es_rd_ghcb_msr(void)
> +{
> +	return __rdmsr(MSR_AMD64_SEV_ES_GHCB);
> +}
> +
> +static inline void sev_es_wr_ghcb_msr(u64 val)
> +{
> +	u32 low, high;
> +
> +	low  = (u32)(val);
> +	high = (u32)(val >> 32);
> +
> +	native_wrmsr(MSR_AMD64_SEV_ES_GHCB, low, high);
> +}

Those copies will go away once you create the common sev.c

> +
> +/* Provides sev_es_terminate() */
> +#include "sev-common-shared.c"
> +
> +void sev_snp_register_ghcb(unsigned long paddr)
> +{
> +	u64 pfn = paddr >> PAGE_SHIFT;
> +	u64 old, val;
> +
> +	/* save the old GHCB MSR */
> +	old = sev_es_rd_ghcb_msr();
> +
> +	/* Issue VMGEXIT */
> +	sev_es_wr_ghcb_msr(GHCB_REGISTER_GPA_REQ_VAL(pfn));
> +	VMGEXIT();
> +
> +	val = sev_es_rd_ghcb_msr();
> +
> +	/* If the response GPA is not ours then abort the guest */
> +	if ((GHCB_SEV_GHCB_RESP_CODE(val) != GHCB_REGISTER_GPA_RESP) ||
> +	    (GHCB_REGISTER_GPA_RESP_VAL(val) != pfn))
> +		sev_es_terminate(GHCB_SEV_ES_REASON_GENERAL_REQUEST);
> +
> +	/* Restore the GHCB MSR value */
> +	sev_es_wr_ghcb_msr(old);
> +}

This is an almost identical copy of the version in compressed/. Move to
the shared file?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette