Re: [PATCH v6 43/60] i386/tdx: Only configure MSR_IA32_UCODE_REV in kvm_init_msrs() for TDs

[Ira Weiny]

On Tue, Nov 05, 2024 at 01:23:51AM -0500, Xiaoyao Li wrote:
> For TDs, only MSR_IA32_UCODE_REV in kvm_init_msrs() can be configured
> by VMM, while the features enumerated/controlled by other MSRs except
> MSR_IA32_UCODE_REV in kvm_init_msrs() are not under control of VMM.

I'm confused by this commit message.  If these features are not under VMM
control with TDX who controls them?  I assume it is the TDX module.  But where
are the qemu hooks to talk to the module?  Are they not needed in qemu at all?

Also, why are the has_msr_* flags true for a TDX TD in the first place?

Ira

> 
> Only configure MSR_IA32_UCODE_REV for TDs.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  target/i386/kvm/kvm.c | 44 ++++++++++++++++++++++---------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 595439f4a4d6..8909fce14909 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -3852,32 +3852,34 @@ static void kvm_init_msrs(X86CPU *cpu)
>      CPUX86State *env = &cpu->env;
>  
>      kvm_msr_buf_reset(cpu);
> -    if (has_msr_arch_capabs) {
> -        kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
> -                          env->features[FEAT_ARCH_CAPABILITIES]);
> -    }
> -
> -    if (has_msr_core_capabs) {
> -        kvm_msr_entry_add(cpu, MSR_IA32_CORE_CAPABILITY,
> -                          env->features[FEAT_CORE_CAPABILITY]);
> -    }
> -
> -    if (has_msr_perf_capabs && cpu->enable_pmu) {
> -        kvm_msr_entry_add_perf(cpu, env->features);
> +
> +    if (!is_tdx_vm()) {
> +        if (has_msr_arch_capabs) {
> +            kvm_msr_entry_add(cpu, MSR_IA32_ARCH_CAPABILITIES,
> +                                env->features[FEAT_ARCH_CAPABILITIES]);
> +        }
> +
> +        if (has_msr_core_capabs) {
> +            kvm_msr_entry_add(cpu, MSR_IA32_CORE_CAPABILITY,
> +                                env->features[FEAT_CORE_CAPABILITY]);
> +        }
> +
> +        if (has_msr_perf_capabs && cpu->enable_pmu) {
> +            kvm_msr_entry_add_perf(cpu, env->features);
> +        }
> +
> +        /*
> +         * Older kernels do not include VMX MSRs in KVM_GET_MSR_INDEX_LIST, 
> but
> +         * all kernels with MSR features should have them.
> +         */
> +        if (kvm_feature_msrs && cpu_has_vmx(env)) {
> +            kvm_msr_entry_add_vmx(cpu, env->features);
> +        }
>      }
>  
>      if (has_msr_ucode_rev) {
>          kvm_msr_entry_add(cpu, MSR_IA32_UCODE_REV, cpu->ucode_rev);
>      }
> -
> -    /*
> -     * Older kernels do not include VMX MSRs in KVM_GET_MSR_INDEX_LIST, but
> -     * all kernels with MSR features should have them.
> -     */
> -    if (kvm_feature_msrs && cpu_has_vmx(env)) {
> -        kvm_msr_entry_add_vmx(cpu, env->features);
> -    }
> -
>      assert(kvm_buf_set_msrs(cpu) == 0);
>  }
>  
> -- 
> 2.34.1
>