On 5/5/21 2:55 PM, Tom Lendacky wrote: > On 5/5/21 1:50 PM, Paolo Bonzini wrote: >> On 05/05/21 20:39, Tom Lendacky wrote: >>> On 5/5/21 11:16 AM, Paolo Bonzini wrote: >>>> On 05/05/21 16:01, Tom Lendacky wrote: >>>>> Boris noticed the below warning when running an SEV-ES guest with >>>>> CONFIG_PROVE_LOCKING=y. >>>>> >>>>> The SRCU lock is released before invoking the vCPU run op where the >>>>> SEV-ES >>>>> support will unmap the GHCB. Is the proper thing to do here to take the >>>>> SRCU lock around the call to kvm_vcpu_unmap() in this case? It does fix >>>>> the issue, but I just want to be sure that I shouldn't, instead, be >>>>> taking >>>>> the memslot lock: >>>> >>>> I would rather avoid having long-lived maps, as I am working on removing >>>> them from the Intel code. However, it seems to me that the GHCB is almost >>>> not used after sev_handle_vmgexit returns? >>> >>> Except for as you pointed out below, things like MMIO and IO. Anything >>> that has to exit to userspace to complete will still need the GHCB mapped >>> when coming back into the kernel if the shared buffer area of the GHCB is >>> being used. >>> >>> Btw, what do you consider long lived maps? Is having a map while context >>> switching back to userspace considered long lived? The GHCB will >>> (possibly) only be mapped on VMEXIT (VMGEXIT) and unmapped on the next >>> VMRUN for the vCPU. An AP reset hold could be a while, though. >> >> Anything that cannot be unmapped in the same function that maps it, >> essentially. >> >>>> 2) upon an AP reset hold exit, the above patch sets the EXITINFO2 field >>>> before the SIPI was received. My understanding is that the processor will >>>> not see the value anyway until it resumes execution, and why would other >>>> vCPUs muck with the AP's GHCB. But I'm not sure if it's okay. >>> >>> As long as the vCPU might not be woken up for any reason other than a >>> SIPI, you can get a way with this. But if it was to be woken up for some >>> other reason (an IPI for some reason?), then you wouldn't want the >>> non-zero value set in the GHCB in advance, because that might cause the >>> vCPU to exit the HLT loop it is in waiting for the actual SIPI. >> >> Ok. Then the best thing to do is to pull sev_es_pre_run to the >> prepare_guest_switch callback. > > A quick test of this failed (VMRUN failure), let me see what is going on > and post back. I couldn't just move sev_es_pre_run() into sev_es_prepare_guest_switch() because of the guest_state_loaded check in svm_prepare_guest_switch(). Renaming sev_es_pre_run() to sev_es_unmap_ghcb() and calling it before the guest_state_loaded check worked nicely. I'll send a patch soon. Thanks, Tom > > Thanks, > Tom > >> >> Paolo >>
