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

[Tom Lendacky <thomas.lendacky@amd.com>]

On 4/7/21 8:35 AM, Brijesh Singh wrote:
> 
> 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.

I believe the current SEV-ES code always sets the GHCB address in the GHCB
MSR before invoking VMGEXIT, so I think you're safe either way. Worth
testing at least.

Thanks,
Tom

> 
> 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
>