Re: [RFC Part1 PATCH 06/13] x86/compressed: rescinds and validate the memory used for the GHCB

From: Brijesh Singh <brijesh.singh@amd.com>
To: Borislav Petkov <bp@alien8.de>
Cc: brijesh.singh@amd.com, linux-kernel@vger.kernel.org,
	x86@kernel.org, kvm@vger.kernel.org, ak@linux.intel.com,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
	"H. Peter Anvin" <hpa@zytor.com>, Tony Luck <tony.luck@intel.com>,
	Dave Hansen <dave.hansen@intel.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	David Rientjes <rientjes@google.com>,
	Sean Christopherson <seanjc@google.com>
Subject: Re: [RFC Part1 PATCH 06/13] x86/compressed: rescinds and validate the memory used for the GHCB
Date: Wed, 7 Apr 2021 08:35:02 -0500	[thread overview]
Message-ID: <9f43f7b2-d9aa-429e-eadd-dc3ea4a34d01@amd.com> (raw)
In-Reply-To: <20210407111604.GA25319@zn.tnic>

On 4/7/21 6:16 AM, Borislav Petkov wrote:
> On Tue, Apr 06, 2021 at 10:47:18AM -0500, Brijesh Singh wrote:
>> Before the GHCB is established the caller does not need to save and
>> restore MSRs. The page_state_change() uses the GHCB MSR protocol and it
>> can be called before and after the GHCB is established hence I am saving
>> and restoring GHCB MSRs.
> I think you need to elaborate on that, maybe with an example. What the
> other sites using the GHCB MSR currently do is:
>
> 1. request by writing it
> 2. read the response
>
> None of them save and restore it.
>
> So why here?

GHCB provides two ways to exit from the guest to the hypervisor. The MSR
protocol and NAEs. The MSR protocol is generally used before the GHCB is
established. After the GHCB is established the guests typically uses the
NAEs. All of the current call sites uses the MSR protocol before the
GHCB is established so they do not need to save and restore the GHCB.
The GHCB is established on the first #VC -
arch/x86/boot/compressed/sev-es.c early_setup_sev_es(). The GHCB page
must a shared page:

early_setup_sev_es()

  set_page_decrypted()

   sev_snp_set_page_shared()

The sev_snp_set_page_shared() called before the GHCB is established.
While exiting from the decompression the sev_es_shutdown_ghcb() is
called to deinit the GHCB.

sev_es_shutdown_ghcb()

  set_page_encrypted()

    sev_snp_set_page_private()

Now that sev_snp_set_private() is called after the GHCB is established.

Since both the sev_snp_set_page_{shared, private}() uses the common
routine to request the page change hence I choose the Page State Change
MSR protocol. In one case the page state request happen before and after
the GHCB is established. We need to save and restore GHCB otherwise will
be loose the previously established GHCB GPA.

If needed then we can avoid the save and restore. The GHCB  provides a
page state change NAE that can be used after the GHCB is established. If
we go with it then code may look like this:

1. Read the GHCB MSR to determine whether the GHCB is established.

2. If GHCB is established then use the page state change NAE

3. If GHCB is not established then use the page state change MSR protocol.

We can eliminate the restore but we still need the rdmsr. The code for
using the NAE page state is going to be a bit larger. Since it is not in
the hot path so I felt we stick with MSR protocol for the page state change.

I am open to suggestions. 

-Brijesh