KVM: x86: Update comments about MSR lists exposed to userspace

  Refresh comments about msrs_to_save, emulated_msrs, and msr_based_features
to remove stale references left behind by commit 2374b7310b66 (KVM:
x86/pmu: Use separate array for defining "PMU MSRs to save"), and to
better reflect the current reality, e.g. emulated_msrs is no longer just
for MSRs that are "kvm-specific".

Reported-by: Binbin Wu <binbin.wu@linux.intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/x86.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)


base-commit: 31b4fc3bc64aadd660c5bfa5178c86a7ba61e0f7
  

On 6/7/2023 8:46 AM, Sean Christopherson wrote:
> Refresh comments about msrs_to_save, emulated_msrs, and msr_based_features
> to remove stale references left behind by commit 2374b7310b66 (KVM:
> x86/pmu: Use separate array for defining "PMU MSRs to save"), and to
> better reflect the current reality, e.g. emulated_msrs is no longer just
> for MSRs that are "kvm-specific".
>
> Reported-by: Binbin Wu <binbin.wu@linux.intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/x86.c | 27 +++++++++++++--------------
>   1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 5ad55ef71433..c77f72cf6dc8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1427,15 +1427,14 @@ int kvm_emulate_rdpmc(struct kvm_vcpu *vcpu)
>   EXPORT_SYMBOL_GPL(kvm_emulate_rdpmc);
>   
>   /*
> - * List of msr numbers which we expose to userspace through KVM_GET_MSRS
> - * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
> - *
> - * The three MSR lists(msrs_to_save, emulated_msrs, msr_based_features)
> - * extract the supported MSRs from the related const lists.
> - * msrs_to_save is selected from the msrs_to_save_all to reflect the
> - * capabilities of the host cpu. This capabilities test skips MSRs that are
> - * kvm-specific. Those are put in emulated_msrs_all; filtering of emulated_msrs
> - * may depend on host virtualization features rather than host cpu features.
> + * The three MSR lists(msrs_to_save, emulated_msrs, msr_based_features) track
> + * the set of MSRs that KVM exposes to userspace through KVM_GET_MSRS,
> + * KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.  msrs_to_save holds MSRs that
> + * require host support, i.e. should be probed via RDMSR.  emulated_msrs holds
> + * MSRs that emulates without strictly requiring host support.
emulates -> emulate/emulated?

BTW, do you think is it better to use msrs_emulated instead of 
emulated_msrs to align the naming style?

> + * msr_based_features holds MSRs that enumerate features, i.e. are effectively
> + * CPUID leafs.  Note, msr_based_features isn't mutually exclusive with
> + * msrs_to_save and emulated_msrs.
>    */
>   
>   static const u32 msrs_to_save_base[] = {
> @@ -1531,11 +1530,11 @@ static const u32 emulated_msrs_all[] = {
>   	MSR_IA32_UCODE_REV,
>   
>   	/*
> -	 * The following list leaves out MSRs whose values are determined
> -	 * by arch/x86/kvm/vmx/nested.c based on CPUID or other MSRs.
> -	 * We always support the "true" VMX control MSRs, even if the host
> -	 * processor does not, so I am putting these registers here rather
> -	 * than in msrs_to_save_all.
> +	 * KVM always supports the "true" VMX control MSRs, even if the host
> +	 * does not.  The VMX MSRs as a whole are considered "emulated" as KVM
> +	 * doesn't strictly require them to exist in the host (ignoring that
> +	 * KVM would refuse to load in the first place if the core set of MSRs
> +	 * aren't supported).
>   	 */
>   	MSR_IA32_VMX_BASIC,
>   	MSR_IA32_VMX_TRUE_PINBASED_CTLS,
>
> base-commit: 31b4fc3bc64aadd660c5bfa5178c86a7ba61e0f7
  

On Wed, Jun 07, 2023, Binbin Wu wrote:
> 
> 
> On 6/7/2023 8:46 AM, Sean Christopherson wrote:
> > Refresh comments about msrs_to_save, emulated_msrs, and msr_based_features
> > to remove stale references left behind by commit 2374b7310b66 (KVM:
> > x86/pmu: Use separate array for defining "PMU MSRs to save"), and to
> > better reflect the current reality, e.g. emulated_msrs is no longer just
> > for MSRs that are "kvm-specific".
> > 
> > Reported-by: Binbin Wu <binbin.wu@linux.intel.com>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >   arch/x86/kvm/x86.c | 27 +++++++++++++--------------
> >   1 file changed, 13 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 5ad55ef71433..c77f72cf6dc8 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1427,15 +1427,14 @@ int kvm_emulate_rdpmc(struct kvm_vcpu *vcpu)
> >   EXPORT_SYMBOL_GPL(kvm_emulate_rdpmc);
> >   /*
> > - * List of msr numbers which we expose to userspace through KVM_GET_MSRS
> > - * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
> > - *
> > - * The three MSR lists(msrs_to_save, emulated_msrs, msr_based_features)
> > - * extract the supported MSRs from the related const lists.
> > - * msrs_to_save is selected from the msrs_to_save_all to reflect the
> > - * capabilities of the host cpu. This capabilities test skips MSRs that are
> > - * kvm-specific. Those are put in emulated_msrs_all; filtering of emulated_msrs
> > - * may depend on host virtualization features rather than host cpu features.
> > + * The three MSR lists(msrs_to_save, emulated_msrs, msr_based_features) track
> > + * the set of MSRs that KVM exposes to userspace through KVM_GET_MSRS,
> > + * KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.  msrs_to_save holds MSRs that
> > + * require host support, i.e. should be probed via RDMSR.  emulated_msrs holds
> > + * MSRs that emulates without strictly requiring host support.
> emulates -> emulate/emulated?

Ah, no, that's supposed to be "that KVM emulates".  I'll fix that up when applying.

> BTW, do you think is it better to use msrs_emulated instead of emulated_msrs
> to align the naming style?

No, "emulated" is used as an adjective that describes each MSR, versus the "to save"
part of msrs_to_save being a description of the list, not each MSR.

If I were going to rename anything, it would be msr_based_features, because that
reads as "MSR-based features", i.e. reads as "a list of features that are tied
to MSRs", whereas the list actually tracks "feature-based MSRs", i.e. "a list of
MSRs that track features".
  

On Tue, 06 Jun 2023 17:46:36 -0700, Sean Christopherson wrote:
> Refresh comments about msrs_to_save, emulated_msrs, and msr_based_features
> to remove stale references left behind by commit 2374b7310b66 (KVM:
> x86/pmu: Use separate array for defining "PMU MSRs to save"), and to
> better reflect the current reality, e.g. emulated_msrs is no longer just
> for MSRs that are "kvm-specific".
> 
> 
> [...]

Applied to kvm-x86 misc, thanks!

[1/1] KVM: x86: Update comments about MSR lists exposed to userspace
      https://github.com/kvm-x86/linux/commit/a30642570855

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes