| Message ID | 20220912075219.70379-1-aik@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kernel] KVM: SVM: Fix function name in comment | expand |
> 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@amd.com> > --- > 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 Also, observed this while reading the code but missed to send a patch. Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com> > * traditional and SEV-ES guests. > */ >
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@amd.com> > --- > 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". 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. */ - - /* 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; }
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@amd.com> >> --- >> 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; > } > >
On Tue, Sep 13, 2022, Alexey Kardashevskiy wrote: > 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@amd.com> > > > --- > > > 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 :) Yeah, having exact function names is nice, but we always seem to end up with goofs like this where a comment gets left behind and then they become stale and confusing. > > 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. Weird consistency nit: KVM refers to VM-Exit as an event and not a thing/action, whereas the APM tends to refer to VMEXIT as a thing, thus the "on VM-Exit" stylization versus "by VMEXIT". Similarly, when talking about the broader event of entering the guest, KVM uses "VM-Enter". VMRUN and VMSAVE on the other hand are instructions and so are "things" in KVM's world. Using the VM-Enter/VM-Exit terminology consistently throughout KVM x86 makes it easy to talk about KVM x86 behavior that is common to both SVM and VMX without getting tripped up on naming differences between the two. So even though it's a little odd odd when looking only at SVM code, using "on VM-Exit" instead of "by VMEXIT" is preferred. > === I want to avoid relying on the APM's arbitrary "Type B" classification. Having to dig up and look at a manual to understand something that's conceptually quite simple is frustrating. Providing references to "Type B" and the table in the changelog is definitely welcome, e.g. so that someone who wants more details/background can easily find that info via via git blame. But for a comment, providing all the information in the comment itself is usually preferable. How about this? Save state that is loaded unconditionally by hardware on VM-Exit for SEV-ES guests, but is not saved via VMRUN or VMSAVE (performed by common SVM code).
Adding Nikunj for his opinion on the reworking (at the very bottom of this mail). On 13/9/22 16:38, Sean Christopherson wrote: > On Tue, Sep 13, 2022, Alexey Kardashevskiy wrote: >> 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@amd.com> >>>> --- >>>> 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 :) > > Yeah, having exact function names is nice, but we always seem to end up with goofs > like this where a comment gets left behind and then they become stale and confusing. > >>> 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. > > Weird consistency nit: KVM refers to VM-Exit as an event and not a thing/action, > whereas the APM tends to refer to VMEXIT as a thing, thus the "on VM-Exit" stylization > versus "by VMEXIT". Similarly, when talking about the broader event of entering > the guest, KVM uses "VM-Enter". > > VMRUN and VMSAVE on the other hand are instructions and so are "things" in KVM's world. > > Using the VM-Enter/VM-Exit terminology consistently throughout KVM x86 makes it easy > to talk about KVM x86 behavior that is common to both SVM and VMX without getting > tripped up on naming differences between the two. So even though it's a little odd > odd when looking only at SVM code, using "on VM-Exit" instead of "by VMEXIT" is > preferred. > >> === > > I want to avoid relying on the APM's arbitrary "Type B" classification. Having to > dig up and look at a manual to understand something that's conceptually quite simple > is frustrating. Providing references to "Type B" and the table in the changelog is > definitely welcome, e.g. so that someone who wants more details/background can easily > find that info via via git blame. But for a comment, providing all the information > in the comment itself is usually preferable. > > How about this? > > Save state that is loaded unconditionally by hardware on VM-Exit for SEV-ES > guests, but is not saved via VMRUN or VMSAVE (performed by common SVM code).
On 20/09/22 14:18, Alexey Kardashevskiy wrote: > Adding Nikunj for his opinion on the reworking (at the very bottom of this mail). > > > On 13/9/22 16:38, Sean Christopherson wrote: >> On Tue, Sep 13, 2022, Alexey Kardashevskiy wrote: >>> 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@amd.com> >>>>> --- >>>>> 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 :) >> >> Yeah, having exact function names is nice, but we always seem to end up with goofs >> like this where a comment gets left behind and then they become stale and confusing. >> >>>> 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. >> >> Weird consistency nit: KVM refers to VM-Exit as an event and not a thing/action, >> whereas the APM tends to refer to VMEXIT as a thing, thus the "on VM-Exit" stylization >> versus "by VMEXIT". Similarly, when talking about the broader event of entering >> the guest, KVM uses "VM-Enter". >> >> VMRUN and VMSAVE on the other hand are instructions and so are "things" in KVM's world. >> >> Using the VM-Enter/VM-Exit terminology consistently throughout KVM x86 makes it easy >> to talk about KVM x86 behavior that is common to both SVM and VMX without getting >> tripped up on naming differences between the two. So even though it's a little odd >> odd when looking only at SVM code, using "on VM-Exit" instead of "by VMEXIT" is >> preferred. >> >>> === >> >> I want to avoid relying on the APM's arbitrary "Type B" classification. Having to >> dig up and look at a manual to understand something that's conceptually quite simple >> is frustrating. Providing references to "Type B" and the table in the changelog is >> definitely welcome, e.g. so that someone who wants more details/background can easily >> find that info via via git blame. How about the following: Save states are classified into three types (APM Volume 2: Table B-3. Swap Types) A: VM-Enter: Host state are saved in host save area VM-Exit: Host state are restored automatically from host save area B: VM-Enter: Host state are _not_ saved to host save area, KVM needs to save required states manually in the host save area VM-Exit: Host state are restored automatically from host save area C: VM-Enter: Host state are _not_ saved to host save area. VM-Exit: Host state are initialized to default(reset) values. Manually save state(type-B) that is loaded unconditionally by hardware on VM-Exit for SEV-ES guests, but is not saved via VMRUN or VMSAVE (performed by common SVM code). >> But for a comment, providing all the information >> in the comment itself is usually preferable. >> >> How about this? >> >> Save state that is loaded unconditionally by hardware on VM-Exit for SEV-ES >> guests, but is not saved via VMRUN or VMSAVE (performed by common SVM code). Regards Nikunj
On Wed, Sep 21, 2022, Nikunj A. Dadhania wrote: > On 20/09/22 14:18, Alexey Kardashevskiy wrote: > > On 13/9/22 16:38, Sean Christopherson wrote: > >> I want to avoid relying on the APM's arbitrary "Type B" classification. Having to > >> dig up and look at a manual to understand something that's conceptually quite simple > >> is frustrating. Providing references to "Type B" and the table in the changelog is > >> definitely welcome, e.g. so that someone who wants more details/background can easily > >> find that info via via git blame. > > How about the following: > > Save states are classified into three types (APM Volume 2: Table B-3. Swap Types) If we do end up with a verbose comment, don't bother with the volume+table details, it will inevitably become stale, even if that takes a few years. What I would take verbatim is the blurb on the classification being determined by "how it is handled by hardware during a world switch", which is the most important details from KVM's perspective. I don't necessarily dislike the idea of capturing the types in a comment, but I also don't think there's a whole lot of value added in doing so. E.g. without the full table, it doesn't help verify the correctness of the code. I'm fine either way though, it's not that large of a comment and it will likely prove helpful to someone at some point in time. > > A: VM-Enter: So this really should be VMRUN, since the table here is specifically covering VMRUN behavior, not KVM's broader VM-Enter sequence. As mentioned earlier in the thread, VM-Enter/VM-Exit is preferred when talking about the sequence, but in this case the table is documenting hardware behavior that is specific to VMRUN. Hrm, and I know that same earlier comment said "on VM-Exit" is preferred, but since this is again specifically talking about the hardware concept of VMEXIT, it's probably best to use that terminology for the table. The short comment I proposed used "on VM-Exit" because it didn't dive as far into the details of hardware's view of things. > Host state are saved in host save area Hmm, I like the APM's approach of avoiding the question of whether or not "Host state" is plural, i.e. omit the "are" (versus "is"?). > VM-Exit: Host state are restored s/restored/loaded Doesn't matter as much here, but when KVM is doing the "saving" for type-B, "restored" can become misleading as KVM isn't strictly required to save its current state. > automatically from host save area s/automatically// KVM could also "automatically" load state on exit, and there's also no need to further qualify that hardware loads are "automatic". Hardware either loads state or it doesn't, e.g. there's no knob that lets KVM say "don't load this state". > B: VM-Enter: Host state are _not_ saved to host save area, KVM needs to save > required states manually in the host save area Drop the KVM line, i.e. keep the "table" purely about the types, and leave the "KVM needs to handle type-B" til the end. E.g. the "manually" part is arguably wrong depending on how VMSAVE is classified. > VM-Exit: Host state are restored automatically from host save area > > C: VM-Enter: Host state are _not_ saved to host save area. > VM-Exit: Host state are initialized to default(reset) values. Please align the comments after the colon, makes it easier on the eyes > Manually save state(type-B) that is loaded unconditionally by hardware on The "save state(type-B)" reads a little odd, maybe "save type-B state"? And the "unconditionally" can be dropped (see "automatically" above), as can the "SEV-ES guests" blurb since this is SEV-ES specific code. Hmm, though it might be worth throwing in a "for SEV-ES guests" in the intro? > VM-Exit for SEV-ES guests, but is not saved via VMRUN or VMSAVE (performed > by common SVM code). Hmm, the comma should be after VMRUN, since type-B state is _all_ state that isn't saved by hardware VMRUN, whereas the key point of this last blurb is to call out that KVM already saves a subset of type-B state via VMSAVE. All in all, this? /* * All host state for SEV-ES guests is categorized into three swap types * based on how it is handled by hardware during a world switch: * * A: VMRUN: Host state saved in host save area * VMEXIT: Host state loaded from host save area * * B: VMRUN: Host state _NOT_ saved in host save area * VMEXIT: Host state loaded from host save area * * C: VMRUN: Host state _NOT_ saved in host save area * VMEXIT: Host state initialized to default(reset) values * * Manually save type-B state, i.e. state that is loaded by VMEXIT but * isn't saved by VMRUN, that isn't already saved by VMSAVE (performed * by common SVM code). */
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. */
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@amd.com> --- arch/x86/kvm/svm/sev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)