On 12/9/22 19:36, Sean Christopherson wrote:
On Mon, Sep 12, 2022, Alexey Kardashevskiy wrote:
A recent renaming patch missed 1 spot, fix it.
This should cause no behavioural change.
Fixes: 23e5092b6e2a ("KVM: SVM: Rename hook implementations to conform to kvm_x86_ops' names")
Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxx>
---
arch/x86/kvm/svm/sev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 28064060413a..3b99a690b60d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3015,7 +3015,7 @@ void sev_es_prepare_switch_to_guest(struct sev_es_save_area *hostsa)
/*
* As an SEV-ES guest, hardware will restore the host state on VMEXIT,
* of which one step is to perform a VMLOAD. KVM performs the
- * corresponding VMSAVE in svm_prepare_guest_switch for both
+ * corresponding VMSAVE in svm_prepare_switch_to_guest for both
* traditional and SEV-ES guests.
*/
Rather than match the rename, what about tweaking the wording to not tie the comment
to the function name, e.g. "VMSAVE in common SVM code".
Although I kinda like the pointer to the caller, it is not that useful
with a single caller and working cscope :)
Even better, This would be a good opportunity to reword this comment to make it more
clear why SEV-ES needs a hook, and to absorb the somewhat useless comments below.
Would something like this be accurate? Please modify and/or add details as necessary.
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 3b99a690b60d..c50c6851aedb 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3013,19 +3013,14 @@ void sev_es_vcpu_reset(struct vcpu_svm *svm)
void sev_es_prepare_switch_to_guest(struct sev_es_save_area *hostsa)
{
/*
- * As an SEV-ES guest, hardware will restore the host state on VMEXIT,
- * of which one step is to perform a VMLOAD. KVM performs the
- * corresponding VMSAVE in svm_prepare_switch_to_guest for both
- * traditional and SEV-ES guests.
+ * Manually save host state that is automatically loaded by hardware on
+ * VM-Exit from SEV-ES guests, but that is not saved by VMSAVE (which is
+ * performed by common SVM code). Hardware unconditionally restores
+ * host state, and so KVM skips manually restoring this state in common
+ * code.
I am new to this arch so not sure :) The AMD spec calls these three
"Type B swaps" from the VMSA's "Table B-3. Swap Types" so may be just say:
===
These are Type B swaps which are not saved by VMSAVE (performed by
common SVM code) but restored by VMEXIT unconditionally.
===
Thanks,
*/
-
- /* XCR0 is restored on VMEXIT, save the current host value */
hostsa->xcr0 = xgetbv(XCR_XFEATURE_ENABLED_MASK);
-
- /* PKRU is restored on VMEXIT, save the current host value */
hostsa->pkru = read_pkru();
-
- /* MSR_IA32_XSS is restored on VMEXIT, save the currnet host value */
hostsa->xss = host_xss;
}
--
Alexey
