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 >