[PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

VM entry into an L2 guest on KVM on Hyper-V fails with the following
splat (stripped for brevity) when running cloud-hypervisor tests.

[ 1481.600386] WARNING: CPU: 4 PID: 7641 at arch/x86/kvm/vmx/nested.c:4563 nested_vmx_vmexit+0x70d/0x790 [kvm_intel]
[ 1481.600427] CPU: 4 PID: 7641 Comm: vcpu2 Not tainted 5.15.0-1008-azure #9-Ubuntu
[ 1481.600429] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 07/22/2021
[ 1481.600430] RIP: 0010:nested_vmx_vmexit+0x70d/0x790 [kvm_intel]
[ 1481.600447] Call Trace:
[ 1481.600449]  <TASK>
[ 1481.600451]  nested_vmx_reflect_vmexit+0x10b/0x440 [kvm_intel]
[ 1481.600457]  __vmx_handle_exit+0xef/0x670 [kvm_intel]
[ 1481.600467]  vmx_handle_exit+0x12/0x50 [kvm_intel]
[ 1481.600472]  vcpu_enter_guest+0x83a/0xfd0 [kvm]
[ 1481.600524]  vcpu_run+0x5e/0x240 [kvm]
[ 1481.600560]  kvm_arch_vcpu_ioctl_run+0xd7/0x550 [kvm]
[ 1481.600597]  kvm_vcpu_ioctl+0x29a/0x6d0 [kvm]
[ 1481.600634]  __x64_sys_ioctl+0x91/0xc0
[ 1481.600637]  do_syscall_64+0x5c/0xc0
[ 1481.600667]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 1481.600670] RIP: 0033:0x7f688becdaff
[ 1481.600686]  </TASK>

As per the comments in arch/x86/kvm/vmx/evmcs.h, TSC multiplier field is
currently not supported in EVMCS. As a result, there is no TSC scaling
support when KVM is running on Hyper-V i.e. kvm_has_tsc_control is
false.

However, in nested_vmx_setup_ctls_msrs(), TSC scaling is exposed to L1.
When L1 tries to launch an L2 guest, vmcs12 has TSC scaling enabled.
This propagates to vmcs02. But KVM doesn't set the TSC multiplier value
because kvm_has_tsc_control is false. Due to this, VM entry for L2 guest
fails. (VM entry fails if "use TSC scaling" is 1 and TSC multiplier is 0.)

To fix, expose TSC scaling to L1 only if kvm_has_tsc_control.

Fixes: d041b5ea93352 ("KVM: nVMX: Enable nested TSC scaling")
Signed-off-by: Anirudh Rayabharam <anrayabh@linux.microsoft.com>
---
 arch/x86/kvm/vmx/nested.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f5cb18e00e78..d773ddc6422b 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6656,6 +6656,9 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 		      msrs->secondary_ctls_low,
 		      msrs->secondary_ctls_high);
 
+	if (!kvm_has_tsc_control)
+		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;
+
 	msrs->secondary_ctls_low = 0;
 	msrs->secondary_ctls_high &=
 		SECONDARY_EXEC_DESC |
@@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 		SECONDARY_EXEC_RDRAND_EXITING |
 		SECONDARY_EXEC_ENABLE_INVPCID |
 		SECONDARY_EXEC_RDSEED_EXITING |
-		SECONDARY_EXEC_XSAVES |
-		SECONDARY_EXEC_TSC_SCALING;
+		SECONDARY_EXEC_XSAVES;
 
 	/*
 	 * We can emulate "VMCS shadowing," even if the hardware
-- 
2.34.1

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Sean Christopherson]

On Mon, Jun 13, 2022, Anirudh Rayabharam wrote:
> As per the comments in arch/x86/kvm/vmx/evmcs.h, TSC multiplier field is

It's not just the comments, it's also the code.  It would be helpful to call out
in the changelog that KVM clears unsupported controls via evmcs_sanitize_exec_ctrls()
when using eVMCS.  

> currently not supported in EVMCS. As a result, there is no TSC scaling
> support when KVM is running on Hyper-V i.e. kvm_has_tsc_control is
> false.
> 
> However, in nested_vmx_setup_ctls_msrs(), TSC scaling is exposed to L1.
> When L1 tries to launch an L2 guest, vmcs12 has TSC scaling enabled.
> This propagates to vmcs02. But KVM doesn't set the TSC multiplier value
> because kvm_has_tsc_control is false. Due to this, VM entry for L2 guest
> fails. (VM entry fails if "use TSC scaling" is 1 and TSC multiplier is 0.)
>
> To fix, expose TSC scaling to L1 only if kvm_has_tsc_control.
> 
> Fixes: d041b5ea93352 ("KVM: nVMX: Enable nested TSC scaling")
> Signed-off-by: Anirudh Rayabharam <anrayabh@linux.microsoft.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index f5cb18e00e78..d773ddc6422b 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -6656,6 +6656,9 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
>  		      msrs->secondary_ctls_low,
>  		      msrs->secondary_ctls_high);
>  
> +	if (!kvm_has_tsc_control)
> +		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;

I would much rather we fix the root of the problem and not play whack-a-mole,
e.g. all of the other controls that aren't supported by eVMCS have the same bug,.

nested_vmx_setup_ctls_msrs() should use vmcs_config to get the base MSR values,
not read the MSRs from hardware.  And it's not just eVMCS, e.g. the manipulation
of VM_{ENTRY,EXIT}_IA32_PERF_GLOBAL_CTRL for a CPU errata isn't handled either.

>  	msrs->secondary_ctls_low = 0;
>  	msrs->secondary_ctls_high &=
>  		SECONDARY_EXEC_DESC |
> @@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
>  		SECONDARY_EXEC_RDRAND_EXITING |
>  		SECONDARY_EXEC_ENABLE_INVPCID |
>  		SECONDARY_EXEC_RDSEED_EXITING |
> -		SECONDARY_EXEC_XSAVES |
> -		SECONDARY_EXEC_TSC_SCALING;
> +		SECONDARY_EXEC_XSAVES;
>  
>  	/*
>  	 * We can emulate "VMCS shadowing," even if the hardware
> -- 
> 2.34.1
> 

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Paolo Bonzini]

On 6/13/22 18:16, Anirudh Rayabharam wrote:
> +	if (!kvm_has_tsc_control)
> +		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;
> +
>   	msrs->secondary_ctls_low = 0;
>   	msrs->secondary_ctls_high &=
>   		SECONDARY_EXEC_DESC |
> @@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
>   		SECONDARY_EXEC_RDRAND_EXITING |
>   		SECONDARY_EXEC_ENABLE_INVPCID |
>   		SECONDARY_EXEC_RDSEED_EXITING |
> -		SECONDARY_EXEC_XSAVES |
> -		SECONDARY_EXEC_TSC_SCALING;
> +		SECONDARY_EXEC_XSAVES;
>   
>   	/*

This is wrong because it _always_ disables SECONDARY_EXEC_TSC_SCALING,
even if kvm_has_tsc_control == true.

That said, I think a better implementation of this patch is to just add
a version of evmcs_sanitize_exec_ctrls that takes a struct
nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like

	evmcs_sanitize_nested_vmx_vsrs(msrs);

Even better (but I cannot "mentally test it" offhand) would be just

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e802f71a9e8d..b3425ce835c5 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1862,7 +1862,7 @@ int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
  		 * sanity checking and refuse to boot. Filter all unsupported
  		 * features out.
  		 */
-		if (!msr_info->host_initiated &&
+		if (static_branch_unlikely(&enable_evmcs) ||
  		    vmx->nested.enlightened_vmcs_enabled)
  			nested_evmcs_filter_control_msr(msr_info->index,
  							&msr_info->data);

I cannot quite understand the host_initiated check, so I'll defer to
Vitaly on why it is needed.  Most likely, removing it would cause some
warnings in QEMU with e.g. "-cpu Haswell,+vmx"; but I think it's a
userspace bug and we should remove that part of the condition.  You
don't need to worry about that part, we'll cross that bridge if the
above patch works for your case.

Thanks,

Paolo

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Sean Christopherson]

On Mon, Jun 13, 2022, Paolo Bonzini wrote:
> On 6/13/22 18:16, Anirudh Rayabharam wrote:
> > +	if (!kvm_has_tsc_control)
> > +		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;
> > +
> >   	msrs->secondary_ctls_low = 0;
> >   	msrs->secondary_ctls_high &=
> >   		SECONDARY_EXEC_DESC |
> > @@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
> >   		SECONDARY_EXEC_RDRAND_EXITING |
> >   		SECONDARY_EXEC_ENABLE_INVPCID |
> >   		SECONDARY_EXEC_RDSEED_EXITING |
> > -		SECONDARY_EXEC_XSAVES |
> > -		SECONDARY_EXEC_TSC_SCALING;
> > +		SECONDARY_EXEC_XSAVES;
> >   	/*
> 
> This is wrong because it _always_ disables SECONDARY_EXEC_TSC_SCALING,
> even if kvm_has_tsc_control == true.
> 
> That said, I think a better implementation of this patch is to just add
> a version of evmcs_sanitize_exec_ctrls that takes a struct
> nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like
> 
> 	evmcs_sanitize_nested_vmx_vsrs(msrs);

Any reason not to use the already sanitized vmcs_config?  I can't think of any
reason why the nested path should blindly use the raw MSR values from hardware.

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Mon, Jun 13, 2022 at 06:49:17PM +0200, Paolo Bonzini wrote:
> On 6/13/22 18:16, Anirudh Rayabharam wrote:
> > +	if (!kvm_has_tsc_control)
> > +		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;
> > +
> >   	msrs->secondary_ctls_low = 0;
> >   	msrs->secondary_ctls_high &=
> >   		SECONDARY_EXEC_DESC |
> > @@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
> >   		SECONDARY_EXEC_RDRAND_EXITING |
> >   		SECONDARY_EXEC_ENABLE_INVPCID |
> >   		SECONDARY_EXEC_RDSEED_EXITING |
> > -		SECONDARY_EXEC_XSAVES |
> > -		SECONDARY_EXEC_TSC_SCALING;
> > +		SECONDARY_EXEC_XSAVES;
> >   	/*
> 
> This is wrong because it _always_ disables SECONDARY_EXEC_TSC_SCALING,
> even if kvm_has_tsc_control == true.

The MSR actually allows 1-setting of the "use TSC scaling" control. So this
line is redundant anyway.

> 
> That said, I think a better implementation of this patch is to just add
> a version of evmcs_sanitize_exec_ctrls that takes a struct
> nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like
> 
> 	evmcs_sanitize_nested_vmx_vsrs(msrs);

Sanitize at the end might not work because I see some cases in
nested_vmx_setup_ctls_msrs() where we want to expose some things to L1
even though the hardware doesn't support it.

> 
> Even better (but I cannot "mentally test it" offhand) would be just
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index e802f71a9e8d..b3425ce835c5 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1862,7 +1862,7 @@ int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		 * sanity checking and refuse to boot. Filter all unsupported
>  		 * features out.
>  		 */
> -		if (!msr_info->host_initiated &&
> +		if (static_branch_unlikely(&enable_evmcs) ||
>  		    vmx->nested.enlightened_vmcs_enabled)
>  			nested_evmcs_filter_control_msr(msr_info->index,
>  							&msr_info->data);

I will try this.

Thanks,

	Anirudh.

> 
> I cannot quite understand the host_initiated check, so I'll defer to
> Vitaly on why it is needed.  Most likely, removing it would cause some
> warnings in QEMU with e.g. "-cpu Haswell,+vmx"; but I think it's a
> userspace bug and we should remove that part of the condition.  You
> don't need to worry about that part, we'll cross that bridge if the
> above patch works for your case.
> 
> Thanks,
> 
> Paolo

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 6/13/22 18:16, Anirudh Rayabharam wrote:
>> +	if (!kvm_has_tsc_control)
>> +		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;
>> +
>>   	msrs->secondary_ctls_low = 0;
>>   	msrs->secondary_ctls_high &=
>>   		SECONDARY_EXEC_DESC |
>> @@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
>>   		SECONDARY_EXEC_RDRAND_EXITING |
>>   		SECONDARY_EXEC_ENABLE_INVPCID |
>>   		SECONDARY_EXEC_RDSEED_EXITING |
>> -		SECONDARY_EXEC_XSAVES |
>> -		SECONDARY_EXEC_TSC_SCALING;
>> +		SECONDARY_EXEC_XSAVES;
>>   
>>   	/*
>
> This is wrong because it _always_ disables SECONDARY_EXEC_TSC_SCALING,
> even if kvm_has_tsc_control == true.
>
> That said, I think a better implementation of this patch is to just add
> a version of evmcs_sanitize_exec_ctrls that takes a struct
> nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like
>
> 	evmcs_sanitize_nested_vmx_vsrs(msrs);
>
> Even better (but I cannot "mentally test it" offhand) would be just
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index e802f71a9e8d..b3425ce835c5 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1862,7 +1862,7 @@ int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		 * sanity checking and refuse to boot. Filter all unsupported
>   		 * features out.
>   		 */
> -		if (!msr_info->host_initiated &&
> +		if (static_branch_unlikely(&enable_evmcs) ||
>   		    vmx->nested.enlightened_vmcs_enabled)
>   			nested_evmcs_filter_control_msr(msr_info->index,
>   							&msr_info->data);
>
> I cannot quite understand the host_initiated check, so I'll defer to
> Vitaly on why it is needed. Most likely, removing it would cause some
> warnings in QEMU with e.g. "-cpu Haswell,+vmx"; but I think it's a
> userspace bug and we should remove that part of the condition. 

I forgot the details, of course, but 31de3d2500e4 says:

```
    With fine grained VMX feature enablement QEMU>=4.2 tries to do KVM_SET_MSRS
    with default (matching CPU model) values and in case eVMCS is also enabled,
    fails.
```

so it certainly was a workaround for QEMU.

-- 
Vitaly

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Paolo Bonzini]

On 6/14/22 06:55, Anirudh Rayabharam wrote:
>> That said, I think a better implementation of this patch is to just add
>> a version of evmcs_sanitize_exec_ctrls that takes a struct
>> nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like
>>
>> 	evmcs_sanitize_nested_vmx_vsrs(msrs);
> Sanitize at the end might not work because I see some cases in
> nested_vmx_setup_ctls_msrs() where we want to expose some things to L1
> even though the hardware doesn't support it.
> 

Yes, but these will never include eVMCS-unsupported features.

Paolo

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:

...

>
> As per the comments in arch/x86/kvm/vmx/evmcs.h, TSC multiplier field is
> currently not supported in EVMCS.

The latest version:
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/datatypes/hv_vmx_enlightened_vmcs

has it, actually. It was missing before (compare with e.g. 6.0b version
here:
https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v6.0b.pdf)

but AFAIR TSC scaling wasn't advertised by genuine Hyper-V either.
Interestingly enough, eVMCS version didn't change when these fields were
added, it is still '1'.

I even have a patch in my stash (attached). I didn't send it out because
it wasn't properly tested with different Hyper-V versions.

-- 
Vitaly


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-KVM-x86-Allow-some-previously-forbidden-controls-whe.patch --]
[-- Type: text/x-patch, Size: 5546 bytes --]

From cb7c34d0c98691cf02a3198ee05cc913300e909b Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Wed, 20 Apr 2022 15:43:37 +0200
Subject: [PATCH RFC] KVM: x86: Allow some previously forbidden controls when
 eVMCS is in use
Content-Type: text/plain

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/vmx/evmcs.h  | 11 ++++-------
 arch/x86/kvm/vmx/nested.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx/evmcs.h b/arch/x86/kvm/vmx/evmcs.h
index ddbdb557cc53..5963c6374db2 100644
--- a/arch/x86/kvm/vmx/evmcs.h
+++ b/arch/x86/kvm/vmx/evmcs.h
@@ -37,16 +37,14 @@ DECLARE_STATIC_KEY_FALSE(enable_evmcs);
  *	EPTP_LIST_ADDRESS               = 0x00002024,
  *	VMREAD_BITMAP                   = 0x00002026,
  *	VMWRITE_BITMAP                  = 0x00002028,
- *
- *	TSC_MULTIPLIER                  = 0x00002032,
  *	PLE_GAP                         = 0x00004020,
  *	PLE_WINDOW                      = 0x00004022,
  *	VMX_PREEMPTION_TIMER_VALUE      = 0x0000482E,
- *      GUEST_IA32_PERF_GLOBAL_CTRL     = 0x00002808,
- *      HOST_IA32_PERF_GLOBAL_CTRL      = 0x00002c04,
  *
- * Currently unsupported in KVM:
- *	GUEST_IA32_RTIT_CTL		= 0x00002814,
+ *	While GUEST_IA32_PERF_GLOBAL_CTRL and HOST_IA32_PERF_GLOBAL_CTRL
+ *	are present in eVMCSv1, Windows 11 still has issues booting when
+ *	VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL/VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL
+ *	are exposed to it, keep them filtered out.
  */
 #define EVMCS1_UNSUPPORTED_PINCTRL (PIN_BASED_POSTED_INTR | \
 				    PIN_BASED_VMX_PREEMPTION_TIMER)
@@ -57,7 +55,6 @@ DECLARE_STATIC_KEY_FALSE(enable_evmcs);
 	 SECONDARY_EXEC_ENABLE_PML |					\
 	 SECONDARY_EXEC_ENABLE_VMFUNC |					\
 	 SECONDARY_EXEC_SHADOW_VMCS |					\
-	 SECONDARY_EXEC_TSC_SCALING |					\
 	 SECONDARY_EXEC_PAUSE_LOOP_EXITING)
 #define EVMCS1_UNSUPPORTED_VMEXIT_CTRL					\
 	(VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL |				\
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 3e2ef5edad4a..4a596973e505 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -1628,6 +1628,10 @@ static void copy_enlightened_to_vmcs12(struct vcpu_vmx *vmx, u32 hv_clean_fields
 		vmcs12->guest_rflags = evmcs->guest_rflags;
 		vmcs12->guest_interruptibility_info =
 			evmcs->guest_interruptibility_info;
+		/*
+		 * Not present in struct vmcs12:
+		 * vmcs12->guest_ssp = evmcs->guest_ssp;
+		 */
 	}
 
 	if (unlikely(!(hv_clean_fields &
@@ -1674,6 +1678,13 @@ static void copy_enlightened_to_vmcs12(struct vcpu_vmx *vmx, u32 hv_clean_fields
 		vmcs12->host_fs_selector = evmcs->host_fs_selector;
 		vmcs12->host_gs_selector = evmcs->host_gs_selector;
 		vmcs12->host_tr_selector = evmcs->host_tr_selector;
+		vmcs12->host_ia32_perf_global_ctrl = evmcs->host_ia32_perf_global_ctrl;
+		/*
+		 * Not present in struct vmcs12:
+		 * vmcs12->host_ia32_s_cet = evmcs->host_ia32_s_cet;
+		 * vmcs12->host_ssp = evmcs->host_ssp;
+		 * vmcs12->host_ia32_int_ssp_table_addr = evmcs->host_ia32_int_ssp_table_addr;
+		 */
 	}
 
 	if (unlikely(!(hv_clean_fields &
@@ -1741,6 +1752,8 @@ static void copy_enlightened_to_vmcs12(struct vcpu_vmx *vmx, u32 hv_clean_fields
 		vmcs12->tsc_offset = evmcs->tsc_offset;
 		vmcs12->virtual_apic_page_addr = evmcs->virtual_apic_page_addr;
 		vmcs12->xss_exit_bitmap = evmcs->xss_exit_bitmap;
+		vmcs12->encls_exiting_bitmap = evmcs->encls_exiting_bitmap;
+		vmcs12->tsc_multiplier = evmcs->tsc_multiplier;
 	}
 
 	if (unlikely(!(hv_clean_fields &
@@ -1788,6 +1801,13 @@ static void copy_enlightened_to_vmcs12(struct vcpu_vmx *vmx, u32 hv_clean_fields
 		vmcs12->guest_bndcfgs = evmcs->guest_bndcfgs;
 		vmcs12->guest_activity_state = evmcs->guest_activity_state;
 		vmcs12->guest_sysenter_cs = evmcs->guest_sysenter_cs;
+		vmcs12->guest_ia32_perf_global_ctrl = evmcs->guest_ia32_perf_global_ctrl;
+		/*
+		 * Not present in struct vmcs12:
+		 * vmcs12->guest_ia32_s_cet = evmcs->guest_ia32_s_cet;
+		 * vmcs12->guest_ia32_lbr_ctl = evmcs->guest_ia32_lbr_ctl;
+		 * vmcs12->guest_ia32_int_ssp_table_addr = evmcs->guest_ia32_int_ssp_table_addr;
+		 */
 	}
 
 	/*
@@ -1890,12 +1910,23 @@ static void copy_vmcs12_to_enlightened(struct vcpu_vmx *vmx)
 	 * evmcs->vm_exit_msr_store_count = vmcs12->vm_exit_msr_store_count;
 	 * evmcs->vm_exit_msr_load_count = vmcs12->vm_exit_msr_load_count;
 	 * evmcs->vm_entry_msr_load_count = vmcs12->vm_entry_msr_load_count;
+	 * evmcs->guest_ia32_perf_global_ctrl = vmcs12->guest_ia32_perf_global_ctrl;
+	 * evmcs->host_ia32_perf_global_ctrl = vmcs12->host_ia32_perf_global_ctrl;
+	 * evmcs->encls_exiting_bitmap = vmcs12->encls_exiting_bitmap;
+	 * evmcs->tsc_multiplier = vmcs12->tsc_multiplier;
 	 *
 	 * Not present in struct vmcs12:
 	 * evmcs->exit_io_instruction_ecx = vmcs12->exit_io_instruction_ecx;
 	 * evmcs->exit_io_instruction_esi = vmcs12->exit_io_instruction_esi;
 	 * evmcs->exit_io_instruction_edi = vmcs12->exit_io_instruction_edi;
 	 * evmcs->exit_io_instruction_eip = vmcs12->exit_io_instruction_eip;
+	 * evmcs->host_ia32_s_cet = vmcs12->host_ia32_s_cet;
+	 * evmcs->host_ssp = vmcs12->host_ssp;
+	 * evmcs->host_ia32_int_ssp_table_addr = vmcs12->host_ia32_int_ssp_table_addr;
+	 * evmcs->guest_ia32_s_cet = vmcs12->guest_ia32_s_cet;
+	 * evmcs->guest_ia32_lbr_ctl = vmcs12->guest_ia32_lbr_ctl;
+	 * evmcs->guest_ia32_int_ssp_table_addr = vmcs12->guest_ia32_int_ssp_table_addr;
+	 * evmcs->guest_ssp = vmcs12->guest_ssp;
 	 */
 
 	evmcs->guest_es_selector = vmcs12->guest_es_selector;
-- 
2.35.3

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:
>
> ...
>
>>
>> As per the comments in arch/x86/kvm/vmx/evmcs.h, TSC multiplier field is
>> currently not supported in EVMCS.
>
> The latest version:
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/datatypes/hv_vmx_enlightened_vmcs
>
> has it, actually. It was missing before (compare with e.g. 6.0b version
> here:
> https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v6.0b.pdf)
>
> but AFAIR TSC scaling wasn't advertised by genuine Hyper-V either.
> Interestingly enough, eVMCS version didn't change when these fields were
> added, it is still '1'.
>
> I even have a patch in my stash (attached). I didn't send it out because
> it wasn't properly tested with different Hyper-V versions.

And of course I forgot a pre-requisite patch which updates 'struct
hv_enlightened_vmcs' to the latest:

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 0a9407dc0859..038e5ef9b4a6 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -559,9 +559,20 @@ struct hv_enlightened_vmcs {
        u64 partition_assist_page;
        u64 padding64_4[4];
        u64 guest_bndcfgs;
-       u64 padding64_5[7];
+       u64 guest_ia32_perf_global_ctrl;
+       u64 guest_ia32_s_cet;
+       u64 guest_ssp;
+       u64 guest_ia32_int_ssp_table_addr;
+       u64 guest_ia32_lbr_ctl;
+       u64 padding64_5[2];
        u64 xss_exit_bitmap;
-       u64 padding64_6[7];
+       u64 host_ia32_perf_global_ctrl;
+       u64 encls_exiting_bitmap;
+       u64 tsc_multiplier;
+       u64 host_ia32_s_cet;
+       u64 host_ssp;
+       u64 host_ia32_int_ssp_table_addr;
+       u64 padding64_6;
 } __packed;
 
 #define HV_VMX_ENLIGHTENED_CLEAN_FIELD_NONE                    0

-- 
Vitaly

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Tue, Jun 14, 2022 at 02:16:00PM +0200, Paolo Bonzini wrote:
> On 6/14/22 06:55, Anirudh Rayabharam wrote:
> > > That said, I think a better implementation of this patch is to just add
> > > a version of evmcs_sanitize_exec_ctrls that takes a struct
> > > nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like
> > > 
> > > 	evmcs_sanitize_nested_vmx_vsrs(msrs);
> > Sanitize at the end might not work because I see some cases in
> > nested_vmx_setup_ctls_msrs() where we want to expose some things to L1
> > even though the hardware doesn't support it.
> > 
> 
> Yes, but these will never include eVMCS-unsupported features.

How are you so sure?

For example, SECONDARY_EXEC_SHADOW_VMCS is unsupported in eVMCS but in
nested_vmx_setup_ctls_msrs() we do:

6675         /*
6676          * We can emulate "VMCS shadowing," even if the hardware
6677          * doesn't support it.
6678          */
6679         msrs->secondary_ctls_high |=
6680                 SECONDARY_EXEC_SHADOW_VMCS;

If we sanitize this out it might cause some regression right?

Thanks!

	Anirudh.
> 
> Paolo

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Tue, Jun 14, 2022 at 10:25:52AM +0530, Anirudh Rayabharam wrote:
> On Mon, Jun 13, 2022 at 06:49:17PM +0200, Paolo Bonzini wrote:
> > On 6/13/22 18:16, Anirudh Rayabharam wrote:
> > > +	if (!kvm_has_tsc_control)
> > > +		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;
> > > +
> > >   	msrs->secondary_ctls_low = 0;
> > >   	msrs->secondary_ctls_high &=
> > >   		SECONDARY_EXEC_DESC |
> > > @@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
> > >   		SECONDARY_EXEC_RDRAND_EXITING |
> > >   		SECONDARY_EXEC_ENABLE_INVPCID |
> > >   		SECONDARY_EXEC_RDSEED_EXITING |
> > > -		SECONDARY_EXEC_XSAVES |
> > > -		SECONDARY_EXEC_TSC_SCALING;
> > > +		SECONDARY_EXEC_XSAVES;
> > >   	/*
> > 
> > This is wrong because it _always_ disables SECONDARY_EXEC_TSC_SCALING,
> > even if kvm_has_tsc_control == true.
> 
> The MSR actually allows 1-setting of the "use TSC scaling" control. So this
> line is redundant anyway.
> 
> > 
> > That said, I think a better implementation of this patch is to just add
> > a version of evmcs_sanitize_exec_ctrls that takes a struct
> > nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like
> > 
> > 	evmcs_sanitize_nested_vmx_vsrs(msrs);
> 
> Sanitize at the end might not work because I see some cases in
> nested_vmx_setup_ctls_msrs() where we want to expose some things to L1
> even though the hardware doesn't support it.
> 
> > 
> > Even better (but I cannot "mentally test it" offhand) would be just
> > 
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index e802f71a9e8d..b3425ce835c5 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -1862,7 +1862,7 @@ int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  		 * sanity checking and refuse to boot. Filter all unsupported
> >  		 * features out.
> >  		 */
> > -		if (!msr_info->host_initiated &&
> > +		if (static_branch_unlikely(&enable_evmcs) ||
> >  		    vmx->nested.enlightened_vmcs_enabled)
> >  			nested_evmcs_filter_control_msr(msr_info->index,
> >  							&msr_info->data);
> 
> I will try this.

This patch fixed the issue for me. But again, this might filter out
things that we wan't to expose to L1 even if not available in hardware.

Thanks,

	Anirudh.

> 
> Thanks,
> 
> 	Anirudh.
> 
> > 
> > I cannot quite understand the host_initiated check, so I'll defer to
> > Vitaly on why it is needed.  Most likely, removing it would cause some
> > warnings in QEMU with e.g. "-cpu Haswell,+vmx"; but I think it's a
> > userspace bug and we should remove that part of the condition.  You
> > don't need to worry about that part, we'll cross that bridge if the
> > above patch works for your case.
> > 
> > Thanks,
> > 
> > Paolo

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Mon, Jun 13, 2022 at 04:57:49PM +0000, Sean Christopherson wrote:
> On Mon, Jun 13, 2022, Paolo Bonzini wrote:
> > On 6/13/22 18:16, Anirudh Rayabharam wrote:
> > > +	if (!kvm_has_tsc_control)
> > > +		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;
> > > +
> > >   	msrs->secondary_ctls_low = 0;
> > >   	msrs->secondary_ctls_high &=
> > >   		SECONDARY_EXEC_DESC |
> > > @@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
> > >   		SECONDARY_EXEC_RDRAND_EXITING |
> > >   		SECONDARY_EXEC_ENABLE_INVPCID |
> > >   		SECONDARY_EXEC_RDSEED_EXITING |
> > > -		SECONDARY_EXEC_XSAVES |
> > > -		SECONDARY_EXEC_TSC_SCALING;
> > > +		SECONDARY_EXEC_XSAVES;
> > >   	/*
> > 
> > This is wrong because it _always_ disables SECONDARY_EXEC_TSC_SCALING,
> > even if kvm_has_tsc_control == true.
> > 
> > That said, I think a better implementation of this patch is to just add
> > a version of evmcs_sanitize_exec_ctrls that takes a struct
> > nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like
> > 
> > 	evmcs_sanitize_nested_vmx_vsrs(msrs);
> 
> Any reason not to use the already sanitized vmcs_config?  I can't think of any
> reason why the nested path should blindly use the raw MSR values from hardware.

vmcs_config has the sanitized exec controls. But how do we construct MSR
values using them?

Thanks,

	Anirudh.

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Sean Christopherson]

On Tue, Jun 14, 2022, Anirudh Rayabharam wrote:
> On Mon, Jun 13, 2022 at 04:57:49PM +0000, Sean Christopherson wrote:
> > On Mon, Jun 13, 2022, Paolo Bonzini wrote:
> > > On 6/13/22 18:16, Anirudh Rayabharam wrote:
> > > > +	if (!kvm_has_tsc_control)
> > > > +		msrs->secondary_ctls_high &= ~SECONDARY_EXEC_TSC_SCALING;
> > > > +
> > > >   	msrs->secondary_ctls_low = 0;
> > > >   	msrs->secondary_ctls_high &=
> > > >   		SECONDARY_EXEC_DESC |
> > > > @@ -6667,8 +6670,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
> > > >   		SECONDARY_EXEC_RDRAND_EXITING |
> > > >   		SECONDARY_EXEC_ENABLE_INVPCID |
> > > >   		SECONDARY_EXEC_RDSEED_EXITING |
> > > > -		SECONDARY_EXEC_XSAVES |
> > > > -		SECONDARY_EXEC_TSC_SCALING;
> > > > +		SECONDARY_EXEC_XSAVES;
> > > >   	/*
> > > 
> > > This is wrong because it _always_ disables SECONDARY_EXEC_TSC_SCALING,
> > > even if kvm_has_tsc_control == true.
> > > 
> > > That said, I think a better implementation of this patch is to just add
> > > a version of evmcs_sanitize_exec_ctrls that takes a struct
> > > nested_vmx_msrs *, and call it at the end of nested_vmx_setup_ctl_msrs like
> > > 
> > > 	evmcs_sanitize_nested_vmx_vsrs(msrs);
> > 
> > Any reason not to use the already sanitized vmcs_config?  I can't think of any
> > reason why the nested path should blindly use the raw MSR values from hardware.
> 
> vmcs_config has the sanitized exec controls. But how do we construct MSR
> values using them?

I was thinking we could use the sanitized controls for the allowed-1 bits, and then
take the required-1 bits from the CPU.  And then if we wanted to avoid the redundant
RDMSRs in a follow-up patch we could add required-1 fields to vmcs_config.

Hastily constructed and compile-tested only, proceed with caution :-)

---
 arch/x86/kvm/vmx/nested.c | 35 ++++++++++++++++++++---------------
 arch/x86/kvm/vmx/nested.h |  2 +-
 arch/x86/kvm/vmx/vmx.c    |  5 ++---
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f5cb18e00e78..67cbb6643efa 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6541,8 +6541,13 @@ static u64 nested_vmx_calc_vmcs_enum_msr(void)
  * bit in the high half is on if the corresponding bit in the control field
  * may be on. See also vmx_control_verify().
  */
-void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
+void nested_vmx_setup_ctls_msrs(struct vmcs_config *vmcs_conf, u32 ept_caps)
 {
+	struct nested_vmx_msrs *msrs = &vmcs_config.nested;
+
+	/* Take the allowed-1 bits from KVM's sanitized VMCS configuration. */
+	u32 ignore_high;
+
 	/*
 	 * Note that as a general rule, the high half of the MSRs (bits in
 	 * the control fields which may be 1) should be initialized by the
@@ -6559,11 +6564,11 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 	 */

 	/* pin-based controls */
-	rdmsr(MSR_IA32_VMX_PINBASED_CTLS,
-		msrs->pinbased_ctls_low,
-		msrs->pinbased_ctls_high);
+	rdmsr(MSR_IA32_VMX_PINBASED_CTLS, msrs->pinbased_ctls_low, ignore_high);
 	msrs->pinbased_ctls_low |=
 		PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
+
+	msrs->pinbased_ctls_high = vmcs_conf->pin_based_exec_ctrl;
 	msrs->pinbased_ctls_high &=
 		PIN_BASED_EXT_INTR_MASK |
 		PIN_BASED_NMI_EXITING |
@@ -6574,12 +6579,11 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 		PIN_BASED_VMX_PREEMPTION_TIMER;

 	/* exit controls */
-	rdmsr(MSR_IA32_VMX_EXIT_CTLS,
-		msrs->exit_ctls_low,
-		msrs->exit_ctls_high);
+	rdmsr(MSR_IA32_VMX_EXIT_CTLS, msrs->exit_ctls_low, ignore_high);
 	msrs->exit_ctls_low =
 		VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR;

+	msrs->exit_ctls_high = vmcs_conf->vmexit_ctrl;
 	msrs->exit_ctls_high &=
 #ifdef CONFIG_X86_64
 		VM_EXIT_HOST_ADDR_SPACE_SIZE |
@@ -6595,11 +6599,11 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 	msrs->exit_ctls_low &= ~VM_EXIT_SAVE_DEBUG_CONTROLS;

 	/* entry controls */
-	rdmsr(MSR_IA32_VMX_ENTRY_CTLS,
-		msrs->entry_ctls_low,
-		msrs->entry_ctls_high);
+	rdmsr(MSR_IA32_VMX_ENTRY_CTLS, msrs->entry_ctls_low, ignore_high);
 	msrs->entry_ctls_low =
 		VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR;
+
+	msrs->entry_ctls_high = vmcs_conf->vmentry_ctrl;
 	msrs->entry_ctls_high &=
 #ifdef CONFIG_X86_64
 		VM_ENTRY_IA32E_MODE |
@@ -6613,11 +6617,11 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 	msrs->entry_ctls_low &= ~VM_ENTRY_LOAD_DEBUG_CONTROLS;

 	/* cpu-based controls */
-	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
-		msrs->procbased_ctls_low,
-		msrs->procbased_ctls_high);
+	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, msrs->procbased_ctls_low, ignore_high);
 	msrs->procbased_ctls_low =
 		CPU_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
+
+	msrs->procbased_ctls_high = vmcs_conf->cpu_based_exec_ctrl;
 	msrs->procbased_ctls_high &=
 		CPU_BASED_INTR_WINDOW_EXITING |
 		CPU_BASED_NMI_WINDOW_EXITING | CPU_BASED_USE_TSC_OFFSETTING |
@@ -6653,10 +6657,11 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 	 */
 	if (msrs->procbased_ctls_high & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)
 		rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
-		      msrs->secondary_ctls_low,
-		      msrs->secondary_ctls_high);
+		      msrs->secondary_ctls_low, ignore_high);

 	msrs->secondary_ctls_low = 0;
+
+	msrs->secondary_ctls_high = vmcs_conf->cpu_based_2nd_exec_ctrl;
 	msrs->secondary_ctls_high &=
 		SECONDARY_EXEC_DESC |
 		SECONDARY_EXEC_ENABLE_RDTSCP |
diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
index c92cea0b8ccc..fae047c6204b 100644
--- a/arch/x86/kvm/vmx/nested.h
+++ b/arch/x86/kvm/vmx/nested.h
@@ -17,7 +17,7 @@ enum nvmx_vmentry_status {
 };

 void vmx_leave_nested(struct kvm_vcpu *vcpu);
-void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps);
+void nested_vmx_setup_ctls_msrs(struct vmcs_config *vmcs_conf, u32 ept_caps);
 void nested_vmx_hardware_unsetup(void);
 __init int nested_vmx_hardware_setup(int (*exit_handlers[])(struct kvm_vcpu *));
 void nested_vmx_set_vmcs_shadowing_bitmap(void);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 9bd86ecccdab..cd0d0ffae0bf 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7139,7 +7139,7 @@ static int __init vmx_check_processor_compat(void)
 	if (setup_vmcs_config(&vmcs_conf, &vmx_cap) < 0)
 		return -EIO;
 	if (nested)
-		nested_vmx_setup_ctls_msrs(&vmcs_conf.nested, vmx_cap.ept);
+		nested_vmx_setup_ctls_msrs(&vmcs_conf, vmx_cap.ept);
 	if (memcmp(&vmcs_config, &vmcs_conf, sizeof(struct vmcs_config)) != 0) {
 		printk(KERN_ERR "kvm: CPU %d feature inconsistency!\n",
 				smp_processor_id());
@@ -8079,8 +8079,7 @@ static __init int hardware_setup(void)
 	setup_default_sgx_lepubkeyhash();

 	if (nested) {
-		nested_vmx_setup_ctls_msrs(&vmcs_config.nested,
-					   vmx_capability.ept);
+		nested_vmx_setup_ctls_msrs(&vmcs_config, vmx_capability.ept);

 		r = nested_vmx_hardware_setup(kvm_vmx_exit_handlers);
 		if (r)

base-commit: b821e4ff9e35a8fc999685e8d44c0644cfeaa228
--

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Paolo Bonzini]

On 6/14/22 14:19, Vitaly Kuznetsov wrote:
> The latest version:
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/datatypes/hv_vmx_enlightened_vmcs
> 
> has it, actually. It was missing before (compare with e.g. 6.0b version
> here:
> https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v6.0b.pdf)
> 
> but AFAIR TSC scaling wasn't advertised by genuine Hyper-V either.
> Interestingly enough, eVMCS version didn't change when these fields were
> added, it is still '1'.
> 
> I even have a patch in my stash (attached). I didn't send it out because
> it wasn't properly tested with different Hyper-V versions.
> 
> -- Vitaly

Anirudh, can you check if Vitaly's patches work for you?

Paolo

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Paolo Bonzini]

On 6/14/22 17:13, Anirudh Rayabharam wrote:
>>> Sanitize at the end might not work because I see some cases in
>>> nested_vmx_setup_ctls_msrs() where we want to expose some things to L1
>>> even though the hardware doesn't support it.
>>
>> Yes, but these will never include eVMCS-unsupported features.
>
> How are you so sure?
> 
> For example, SECONDARY_EXEC_SHADOW_VMCS is unsupported in eVMCS but in
> nested_vmx_setup_ctls_msrs() we do:
> 
> 6675         /*
> 6676          * We can emulate "VMCS shadowing," even if the hardware
> 6677          * doesn't support it.
> 6678          */
> 6679         msrs->secondary_ctls_high |=
> 6680                 SECONDARY_EXEC_SHADOW_VMCS;
> 
> If we sanitize this out it might cause some regression right?

Yes, you're right, shadow VMCS is special: it is not supported by 
enlightened VMCS, but it is emulated rather than virtualized. 
Therefore, if L1 does not use the enlightened VMCS, it can indeed use 
shadow VMCS.

Paolo

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Tue, Jun 14, 2022 at 07:20:34PM +0200, Paolo Bonzini wrote:
> On 6/14/22 14:19, Vitaly Kuznetsov wrote:
> > The latest version:
> > https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/datatypes/hv_vmx_enlightened_vmcs
> > 
> > has it, actually. It was missing before (compare with e.g. 6.0b version
> > here:
> > https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v6.0b.pdf)
> > 
> > but AFAIR TSC scaling wasn't advertised by genuine Hyper-V either.
> > Interestingly enough, eVMCS version didn't change when these fields were
> > added, it is still '1'.
> > 
> > I even have a patch in my stash (attached). I didn't send it out because
> > it wasn't properly tested with different Hyper-V versions.
> > 
> > -- Vitaly
> 
> Anirudh, can you check if Vitaly's patches work for you?

I will check it. But I wonder if they fit the criteria for inclusion in
stable trees...

It is important for the fix to land in the stable trees since this issue
is a regression that was introduced _after_ 5.13. (I probably should've
mentioned this in the changelog.)

Thanks,

	Anirudh.

> 
> Paolo

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:

> On Tue, Jun 14, 2022 at 07:20:34PM +0200, Paolo Bonzini wrote:
>> On 6/14/22 14:19, Vitaly Kuznetsov wrote:
>> > The latest version:
>> > https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/datatypes/hv_vmx_enlightened_vmcs
>> > 
>> > has it, actually. It was missing before (compare with e.g. 6.0b version
>> > here:
>> > https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v6.0b.pdf)
>> > 
>> > but AFAIR TSC scaling wasn't advertised by genuine Hyper-V either.
>> > Interestingly enough, eVMCS version didn't change when these fields were
>> > added, it is still '1'.
>> > 
>> > I even have a patch in my stash (attached). I didn't send it out because
>> > it wasn't properly tested with different Hyper-V versions.
>> > 
>> > -- Vitaly
>> 
>> Anirudh, can you check if Vitaly's patches work for you?
>
> I will check it. But I wonder if they fit the criteria for inclusion in
> stable trees...
>
> It is important for the fix to land in the stable trees since this issue
> is a regression that was introduced _after_ 5.13. (I probably should've
> mentioned this in the changelog.)
>

Personally, I see no problem with splitting off TscMultiplier part from
my patch and marking it for stable@ fixing d041b5ea93352. I'm going to
run some tests too.

-- 
Vitaly

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

Vitaly Kuznetsov <vkuznets@redhat.com> writes:

> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>
>> Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:
>>
>> ...
>>
>>>
>>> As per the comments in arch/x86/kvm/vmx/evmcs.h, TSC multiplier field is
>>> currently not supported in EVMCS.
>>
>> The latest version:
>> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/datatypes/hv_vmx_enlightened_vmcs
>>
>> has it, actually. It was missing before (compare with e.g. 6.0b version
>> here:
>> https://github.com/MicrosoftDocs/Virtualization-Documentation/raw/live/tlfs/Hypervisor%20Top%20Level%20Functional%20Specification%20v6.0b.pdf)
>>
>> but AFAIR TSC scaling wasn't advertised by genuine Hyper-V either.
>> Interestingly enough, eVMCS version didn't change when these fields were
>> added, it is still '1'.
>>
>> I even have a patch in my stash (attached). I didn't send it out because
>> it wasn't properly tested with different Hyper-V versions.
>
> And of course I forgot a pre-requisite patch which updates 'struct
> hv_enlightened_vmcs' to the latest:
>

The good news is that TscMultiplies seems to work fine for me, at least
with an Azure Dv5 instance where I can see Tsc scaling exposed. The bad
news is that a few more patches are needed:

1) Fix 'struct hv_enlightened_vmcs' definition:
https://lore.kernel.org/kvm/20220613133922.2875594-20-vkuznets@redhat.com/

2) Define VMCS-to-EVMCS conversion for the new fields :

diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
index 6a61b1ae7942..707a8de11802 100644
--- a/arch/x86/kvm/vmx/evmcs.c
+++ b/arch/x86/kvm/vmx/evmcs.c
@@ -28,6 +28,8 @@ const struct evmcs_field vmcs_field_to_evmcs_1[] = {
                     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
        EVMCS1_FIELD(HOST_IA32_EFER, host_ia32_efer,
                     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
+       EVMCS1_FIELD(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl,
+                    HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
        EVMCS1_FIELD(HOST_CR0, host_cr0,
                     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
        EVMCS1_FIELD(HOST_CR3, host_cr3,
@@ -78,6 +80,8 @@ const struct evmcs_field vmcs_field_to_evmcs_1[] = {
                     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
        EVMCS1_FIELD(GUEST_IA32_EFER, guest_ia32_efer,
                     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
+       EVMCS1_FIELD(GUEST_IA32_PERF_GLOBAL_CTRL, guest_ia32_perf_global_ctrl,
+                    HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
        EVMCS1_FIELD(GUEST_PDPTR0, guest_pdptr0,
                     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
        EVMCS1_FIELD(GUEST_PDPTR1, guest_pdptr1,
@@ -126,24 +130,47 @@ const struct evmcs_field vmcs_field_to_evmcs_1[] = {
                     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
        EVMCS1_FIELD(XSS_EXIT_BITMAP, xss_exit_bitmap,
                     HV_VMX_ENLIGHTENED_CLEAN_FIELD_CONTROL_GRP2),
+       EVMCS1_FIELD(ENCLS_EXITING_BITMAP, encls_exiting_bitmap,
+                    HV_VMX_ENLIGHTENED_CLEAN_FIELD_CONTROL_GRP2),
+       EVMCS1_FIELD(TSC_MULTIPLIER, tsc_multiplier,
+                    HV_VMX_ENLIGHTENED_CLEAN_FIELD_CONTROL_GRP2),

...

so it is becoming more and more complicated to assemble for testing. Let
me finish my testing and I'll put a series together and send it out to
simplify the process. Stay tuned!

-- 
Vitaly

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

Sean Christopherson <seanjc@google.com> writes:

> On Tue, Jun 14, 2022, Anirudh Rayabharam wrote:
>> On Mon, Jun 13, 2022 at 04:57:49PM +0000, Sean Christopherson wrote:

...

>> > 
>> > Any reason not to use the already sanitized vmcs_config?  I can't think of any
>> > reason why the nested path should blindly use the raw MSR values from hardware.
>> 
>> vmcs_config has the sanitized exec controls. But how do we construct MSR
>> values using them?
>
> I was thinking we could use the sanitized controls for the allowed-1 bits, and then
> take the required-1 bits from the CPU.  And then if we wanted to avoid the redundant
> RDMSRs in a follow-up patch we could add required-1 fields to vmcs_config.
>
> Hastily constructed and compile-tested only, proceed with caution :-)

Independently from "[PATCH 00/11] KVM: VMX: Support TscScaling and
EnclsExitingBitmap whith eVMCS" which is supposed to fix the particular
TSC scaling issue, I like the idea to make nested_vmx_setup_ctls_msrs()
use both allowed-1 and required-1 bits from vmcs_config. I'll pick up
the suggested patch and try to construct something for required-1 bits.

Thanks!

-- 
Vitaly

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Wed, Jun 22, 2022 at 10:00:29AM +0200, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > On Tue, Jun 14, 2022, Anirudh Rayabharam wrote:
> >> On Mon, Jun 13, 2022 at 04:57:49PM +0000, Sean Christopherson wrote:
> 
> ...
> 
> >> > 
> >> > Any reason not to use the already sanitized vmcs_config?  I can't think of any
> >> > reason why the nested path should blindly use the raw MSR values from hardware.
> >> 
> >> vmcs_config has the sanitized exec controls. But how do we construct MSR
> >> values using them?
> >
> > I was thinking we could use the sanitized controls for the allowed-1 bits, and then
> > take the required-1 bits from the CPU.  And then if we wanted to avoid the redundant
> > RDMSRs in a follow-up patch we could add required-1 fields to vmcs_config.
> >
> > Hastily constructed and compile-tested only, proceed with caution :-)
> 
> Independently from "[PATCH 00/11] KVM: VMX: Support TscScaling and
> EnclsExitingBitmap whith eVMCS" which is supposed to fix the particular
> TSC scaling issue, I like the idea to make nested_vmx_setup_ctls_msrs()
> use both allowed-1 and required-1 bits from vmcs_config. I'll pick up
> the suggested patch and try to construct something for required-1 bits.

I tried this patch today but it causes some regression which causes
/dev/kvm to be unavailable in L1. I didn't get a chance to look into it
closely but I am guessing it has something to do with the fact that
vmcs_config reflects the config that L0 chose to use rather than what is
available to use. So constructing allowed-1 MSR bits based on what bits
are set in exec controls maybe isn't correct.


Thanks!

	- Anirudh.

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:

> On Wed, Jun 22, 2022 at 10:00:29AM +0200, Vitaly Kuznetsov wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>> 
>> > On Tue, Jun 14, 2022, Anirudh Rayabharam wrote:
>> >> On Mon, Jun 13, 2022 at 04:57:49PM +0000, Sean Christopherson wrote:
>> 
>> ...
>> 
>> >> > 
>> >> > Any reason not to use the already sanitized vmcs_config?  I can't think of any
>> >> > reason why the nested path should blindly use the raw MSR values from hardware.
>> >> 
>> >> vmcs_config has the sanitized exec controls. But how do we construct MSR
>> >> values using them?
>> >
>> > I was thinking we could use the sanitized controls for the allowed-1 bits, and then
>> > take the required-1 bits from the CPU.  And then if we wanted to avoid the redundant
>> > RDMSRs in a follow-up patch we could add required-1 fields to vmcs_config.
>> >
>> > Hastily constructed and compile-tested only, proceed with caution :-)
>> 
>> Independently from "[PATCH 00/11] KVM: VMX: Support TscScaling and
>> EnclsExitingBitmap whith eVMCS" which is supposed to fix the particular
>> TSC scaling issue, I like the idea to make nested_vmx_setup_ctls_msrs()
>> use both allowed-1 and required-1 bits from vmcs_config. I'll pick up
>> the suggested patch and try to construct something for required-1 bits.
>
> I tried this patch today but it causes some regression which causes
> /dev/kvm to be unavailable in L1. I didn't get a chance to look into it
> closely but I am guessing it has something to do with the fact that
> vmcs_config reflects the config that L0 chose to use rather than what is
> available to use. So constructing allowed-1 MSR bits based on what bits
> are set in exec controls maybe isn't correct.

I've tried to pick it up but it's actually much harder than I think. The
patch has some minor issues ('&vmcs_config.nested' needs to be switched
to '&vmcs_conf->nested' in nested_vmx_setup_ctls_msrs()), but the main
problem is that the set of controls nested_vmx_setup_ctls_msrs() needs
is NOT a subset of vmcs_config (setup_vmcs_config()). I was able to
identify at least:

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 5e14e4c40007..8076352174ad 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2483,8 +2483,14 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
              CPU_BASED_INVLPG_EXITING |
              CPU_BASED_RDPMC_EXITING;
 
-       opt = CPU_BASED_TPR_SHADOW |
+       opt = CPU_BASED_INTR_WINDOW_EXITING |
+             CPU_BASED_NMI_WINDOW_EXITING |
+             CPU_BASED_TPR_SHADOW |
+             CPU_BASED_USE_IO_BITMAPS |
              CPU_BASED_USE_MSR_BITMAPS |
+             CPU_BASED_MONITOR_TRAP_FLAG |
+             CPU_BASED_RDTSC_EXITING |
+             CPU_BASED_PAUSE_EXITING |
              CPU_BASED_ACTIVATE_SECONDARY_CONTROLS |
              CPU_BASED_ACTIVATE_TERTIARY_CONTROLS;
        if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PROCBASED_CTLS,
@@ -2582,6 +2588,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 #endif
        opt = VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL |
              VM_EXIT_LOAD_IA32_PAT |
+             VM_EXIT_SAVE_IA32_PAT |
              VM_EXIT_LOAD_IA32_EFER |
              VM_EXIT_CLEAR_BNDCFGS |
              VM_EXIT_PT_CONCEAL_PIP |
@@ -2604,7 +2611,11 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
                _pin_based_exec_control &= ~PIN_BASED_POSTED_INTR;
 
        min = VM_ENTRY_LOAD_DEBUG_CONTROLS;
-       opt = VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL |
+       opt =
+#ifdef CONFIG_X86_64
+             VM_ENTRY_IA32E_MODE |
+#endif
+             VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL |
              VM_ENTRY_LOAD_IA32_PAT |
              VM_ENTRY_LOAD_IA32_EFER |
              VM_ENTRY_LOAD_BNDCFGS |

but it is 1) not sufficient because some controls are smartly filtered
out just because we don't want them for L1 -- and this doesn't mean that
L2 doesn't need them and 2) because if we add some 'opt' controls to
setup_vmcs_config() we need to filter them out somewhere else.

I'm starting to think we may just want to store raw VMX MSR values in
vmcs_config first, then sanitize them (eVMCS, vmx preemtoion timer bug,
perf_ctrl bug,..) and then do the adjust_vmx_controls() magic. 

I'm not giving up yet but don't expect something small and backportable
to stable :-) 

-- 
Vitaly

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Wed, Jun 22, 2022 at 04:35:27PM +0200, Vitaly Kuznetsov wrote:
> Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:
> 
> > On Wed, Jun 22, 2022 at 10:00:29AM +0200, Vitaly Kuznetsov wrote:
> >> Sean Christopherson <seanjc@google.com> writes:
> >> 
> >> > On Tue, Jun 14, 2022, Anirudh Rayabharam wrote:
> >> >> On Mon, Jun 13, 2022 at 04:57:49PM +0000, Sean Christopherson wrote:
> >> 
> >> ...
> >> 
> >> >> > 
> >> >> > Any reason not to use the already sanitized vmcs_config?  I can't think of any
> >> >> > reason why the nested path should blindly use the raw MSR values from hardware.
> >> >> 
> >> >> vmcs_config has the sanitized exec controls. But how do we construct MSR
> >> >> values using them?
> >> >
> >> > I was thinking we could use the sanitized controls for the allowed-1 bits, and then
> >> > take the required-1 bits from the CPU.  And then if we wanted to avoid the redundant
> >> > RDMSRs in a follow-up patch we could add required-1 fields to vmcs_config.
> >> >
> >> > Hastily constructed and compile-tested only, proceed with caution :-)
> >> 
> >> Independently from "[PATCH 00/11] KVM: VMX: Support TscScaling and
> >> EnclsExitingBitmap whith eVMCS" which is supposed to fix the particular
> >> TSC scaling issue, I like the idea to make nested_vmx_setup_ctls_msrs()
> >> use both allowed-1 and required-1 bits from vmcs_config. I'll pick up
> >> the suggested patch and try to construct something for required-1 bits.
> >
> > I tried this patch today but it causes some regression which causes
> > /dev/kvm to be unavailable in L1. I didn't get a chance to look into it
> > closely but I am guessing it has something to do with the fact that
> > vmcs_config reflects the config that L0 chose to use rather than what is
> > available to use. So constructing allowed-1 MSR bits based on what bits
> > are set in exec controls maybe isn't correct.
> 
> I've tried to pick it up but it's actually much harder than I think. The
> patch has some minor issues ('&vmcs_config.nested' needs to be switched
> to '&vmcs_conf->nested' in nested_vmx_setup_ctls_msrs()), but the main
> problem is that the set of controls nested_vmx_setup_ctls_msrs() needs
> is NOT a subset of vmcs_config (setup_vmcs_config()). I was able to
> identify at least:
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 5e14e4c40007..8076352174ad 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2483,8 +2483,14 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>               CPU_BASED_INVLPG_EXITING |
>               CPU_BASED_RDPMC_EXITING;
>  
> -       opt = CPU_BASED_TPR_SHADOW |
> +       opt = CPU_BASED_INTR_WINDOW_EXITING |
> +             CPU_BASED_NMI_WINDOW_EXITING |
> +             CPU_BASED_TPR_SHADOW |
> +             CPU_BASED_USE_IO_BITMAPS |
>               CPU_BASED_USE_MSR_BITMAPS |
> +             CPU_BASED_MONITOR_TRAP_FLAG |
> +             CPU_BASED_RDTSC_EXITING |
> +             CPU_BASED_PAUSE_EXITING |
>               CPU_BASED_ACTIVATE_SECONDARY_CONTROLS |
>               CPU_BASED_ACTIVATE_TERTIARY_CONTROLS;
>         if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_PROCBASED_CTLS,
> @@ -2582,6 +2588,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>  #endif
>         opt = VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL |
>               VM_EXIT_LOAD_IA32_PAT |
> +             VM_EXIT_SAVE_IA32_PAT |
>               VM_EXIT_LOAD_IA32_EFER |
>               VM_EXIT_CLEAR_BNDCFGS |
>               VM_EXIT_PT_CONCEAL_PIP |
> @@ -2604,7 +2611,11 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>                 _pin_based_exec_control &= ~PIN_BASED_POSTED_INTR;
>  
>         min = VM_ENTRY_LOAD_DEBUG_CONTROLS;
> -       opt = VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL |
> +       opt =
> +#ifdef CONFIG_X86_64
> +             VM_ENTRY_IA32E_MODE |
> +#endif
> +             VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL |
>               VM_ENTRY_LOAD_IA32_PAT |
>               VM_ENTRY_LOAD_IA32_EFER |
>               VM_ENTRY_LOAD_BNDCFGS |
> 
> but it is 1) not sufficient because some controls are smartly filtered
> out just because we don't want them for L1 -- and this doesn't mean that
> L2 doesn't need them and 2) because if we add some 'opt' controls to
> setup_vmcs_config() we need to filter them out somewhere else.
> 
> I'm starting to think we may just want to store raw VMX MSR values in
> vmcs_config first, then sanitize them (eVMCS, vmx preemtoion timer bug,
> perf_ctrl bug,..) and then do the adjust_vmx_controls() magic. 
> 
> I'm not giving up yet but don't expect something small and backportable
> to stable :-) 

How about we do something simple like the patch below to start with?
This will easily apply to stable and we can continue improving upon
it with follow up patches on mainline.

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f5cb18e00e78..f88d748c7cc6 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6564,6 +6564,10 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 		msrs->pinbased_ctls_high);
 	msrs->pinbased_ctls_low |=
 		PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR;
+#if IS_ENABLED(CONFIG_HYPERV)
+	if (static_branch_unlikely(&enable_evmcs))
+		msrs->pinbased_ctls_high &= ~EVMCS1_UNSUPPORTED_PINCTRL;
+#endif
 	msrs->pinbased_ctls_high &=
 		PIN_BASED_EXT_INTR_MASK |
 		PIN_BASED_NMI_EXITING |
@@ -6580,6 +6584,10 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 	msrs->exit_ctls_low =
 		VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR;
 
+#if IS_ENABLED(CONFIG_HYPERV)
+	if (static_branch_unlikely(&enable_evmcs))
+		msrs->exit_ctls_high &= ~EVMCS1_UNSUPPORTED_VMEXIT_CTRL;
+#endif
 	msrs->exit_ctls_high &=
 #ifdef CONFIG_X86_64
 		VM_EXIT_HOST_ADDR_SPACE_SIZE |
@@ -6600,6 +6608,10 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 		msrs->entry_ctls_high);
 	msrs->entry_ctls_low =
 		VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR;
+#if IS_ENABLED(CONFIG_HYPERV)
+	if (static_branch_unlikely(&enable_evmcs))
+		msrs->entry_ctls_high &= ~EVMCS1_UNSUPPORTED_VMENTRY_CTRL;
+#endif
 	msrs->entry_ctls_high &=
 #ifdef CONFIG_X86_64
 		VM_ENTRY_IA32E_MODE |
@@ -6657,6 +6669,10 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 		      msrs->secondary_ctls_high);
 
 	msrs->secondary_ctls_low = 0;
+#if IS_ENABLED(CONFIG_HYPERV)
+	if (static_branch_unlikely(&enable_evmcs))
+		msrs->secondary_ctls_high &= ~EVMCS1_UNSUPPORTED_2NDEXEC;
+#endif
 	msrs->secondary_ctls_high &=
 		SECONDARY_EXEC_DESC |
 		SECONDARY_EXEC_ENABLE_RDTSCP |

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:

> On Wed, Jun 22, 2022 at 04:35:27PM +0200, Vitaly Kuznetsov wrote:

...

>> 
>> I've tried to pick it up but it's actually much harder than I think. The
>> patch has some minor issues ('&vmcs_config.nested' needs to be switched
>> to '&vmcs_conf->nested' in nested_vmx_setup_ctls_msrs()), but the main
>> problem is that the set of controls nested_vmx_setup_ctls_msrs() needs
>> is NOT a subset of vmcs_config (setup_vmcs_config()). I was able to
>> identify at least:

...

I've jsut sent "[PATCH RFC v1 00/10] KVM: nVMX: Use vmcs_config for
setting up nested VMX MSRs" which implements Sean's suggestion. Hope
this is the way to go for mainline.

>
> How about we do something simple like the patch below to start with?
> This will easily apply to stable and we can continue improving upon
> it with follow up patches on mainline.
>

Personally, I'm not against this for @stable. Alternatively, in case the
only observed issue is with TSC scaling, we can add support for it for
KVM-on-Hyper-V but not for Hyper-V-on-KVM (a small subset of "[PATCH
00/11] KVM: VMX: Support TscScaling and EnclsExitingBitmap whith
eVMCS"). I can prepare patches if needed.

-- 
Vitaly

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Wed, Jun 22, 2022 at 06:48:50PM +0200, Vitaly Kuznetsov wrote:
> Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:
> 
> > On Wed, Jun 22, 2022 at 04:35:27PM +0200, Vitaly Kuznetsov wrote:
> 
> ...
> 
> >> 
> >> I've tried to pick it up but it's actually much harder than I think. The
> >> patch has some minor issues ('&vmcs_config.nested' needs to be switched
> >> to '&vmcs_conf->nested' in nested_vmx_setup_ctls_msrs()), but the main
> >> problem is that the set of controls nested_vmx_setup_ctls_msrs() needs
> >> is NOT a subset of vmcs_config (setup_vmcs_config()). I was able to
> >> identify at least:
> 
> ...
> 
> I've jsut sent "[PATCH RFC v1 00/10] KVM: nVMX: Use vmcs_config for
> setting up nested VMX MSRs" which implements Sean's suggestion. Hope
> this is the way to go for mainline.
> 
> >
> > How about we do something simple like the patch below to start with?
> > This will easily apply to stable and we can continue improving upon
> > it with follow up patches on mainline.
> >
> 
> Personally, I'm not against this for @stable. Alternatively, in case the

I think it's a good intermediate fix for mainline too. It is easier to land
it in stable if it already exists in mainline. It can stay in mainline
until your series lands and replaces it with the vmcs_config approach.

What do you think?

> only observed issue is with TSC scaling, we can add support for it for
> KVM-on-Hyper-V but not for Hyper-V-on-KVM (a small subset of "[PATCH
> 00/11] KVM: VMX: Support TscScaling and EnclsExitingBitmap whith
> eVMCS"). I can prepare patches if needed.

Will it fit in stable's 100 line rule?

Thanks!

	- Anirudh.

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Vitaly Kuznetsov]

[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]

Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:

> On Wed, Jun 22, 2022 at 06:48:50PM +0200, Vitaly Kuznetsov wrote:
>> Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:
>> 
>> > On Wed, Jun 22, 2022 at 04:35:27PM +0200, Vitaly Kuznetsov wrote:
>> 
>> ...
>> 
>> >> 
>> >> I've tried to pick it up but it's actually much harder than I think. The
>> >> patch has some minor issues ('&vmcs_config.nested' needs to be switched
>> >> to '&vmcs_conf->nested' in nested_vmx_setup_ctls_msrs()), but the main
>> >> problem is that the set of controls nested_vmx_setup_ctls_msrs() needs
>> >> is NOT a subset of vmcs_config (setup_vmcs_config()). I was able to
>> >> identify at least:
>> 
>> ...
>> 
>> I've jsut sent "[PATCH RFC v1 00/10] KVM: nVMX: Use vmcs_config for
>> setting up nested VMX MSRs" which implements Sean's suggestion. Hope
>> this is the way to go for mainline.
>> 
>> >
>> > How about we do something simple like the patch below to start with?
>> > This will easily apply to stable and we can continue improving upon
>> > it with follow up patches on mainline.
>> >
>> 
>> Personally, I'm not against this for @stable. Alternatively, in case the
>
> I think it's a good intermediate fix for mainline too. It is easier to land
> it in stable if it already exists in mainline. It can stay in mainline
> until your series lands and replaces it with the vmcs_config approach.
>
> What do you think?
>

Paolo's call but personally I think both series can make 5.20 so there's
no need for an intermediate solution.

>> only observed issue is with TSC scaling, we can add support for it for
>> KVM-on-Hyper-V but not for Hyper-V-on-KVM (a small subset of "[PATCH
>> 00/11] KVM: VMX: Support TscScaling and EnclsExitingBitmap whith
>> eVMCS"). I can prepare patches if needed.
>
> Will it fit in stable's 100 line rule?
>

Yes, please take a look at the attached patches (5.18.y based). First 3
are identical to what I've sent for mainline, the last one is reduced to
only support TSC scaling for KVM on Hyper-V (but not Hyper-V on
KVM). Compile tested only, proceed with caution)

-- 
Vitaly


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-x86-hyperv-Fix-struct-hv_enlightened_vmcs-definition.patch --]
[-- Type: text/x-patch, Size: 2146 bytes --]

From 3057bc241d70152df5f82cfc1fa03d11c91fb48a Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Mon, 13 Jun 2022 15:39:02 +0200
Subject: [PATCH 1/4] x86/hyperv: Fix 'struct hv_enlightened_vmcs' definition
Content-Type: text/plain

Section 1.9 of TLFS v6.0b says:

"All structures are padded in such a way that fields are aligned
naturally (that is, an 8-byte field is aligned to an offset of 8 bytes
and so on)".

'struct enlightened_vmcs' has a glitch:

...
        struct {
                u32                nested_flush_hypercall:1; /*   836: 0  4 */
                u32                msr_bitmap:1;         /*   836: 1  4 */
                u32                reserved:30;          /*   836: 2  4 */
        } hv_enlightenments_control;                     /*   836     4 */
        u32                        hv_vp_id;             /*   840     4 */
        u64                        hv_vm_id;             /*   844     8 */
        u64                        partition_assist_page; /*   852     8 */
...

And the observed values in 'partition_assist_page' make no sense at
all. Fix the layout by padding the structure properly.

Fixes: 68d1eb72ee99 ("x86/hyper-v: define struct hv_enlightened_vmcs and clean field bits")
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/include/asm/hyperv-tlfs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 0a9407dc0859..6f0acc45e67a 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -546,7 +546,7 @@ struct hv_enlightened_vmcs {
 	u64 guest_rip;
 
 	u32 hv_clean_fields;
-	u32 hv_padding_32;
+	u32 padding32_1;
 	u32 hv_synthetic_controls;
 	struct {
 		u32 nested_flush_hypercall:1;
@@ -554,7 +554,7 @@ struct hv_enlightened_vmcs {
 		u32 reserved:30;
 	}  __packed hv_enlightenments_control;
 	u32 hv_vp_id;
-
+	u32 padding32_2;
 	u64 hv_vm_id;
 	u64 partition_assist_page;
 	u64 padding64_4[4];
-- 
2.35.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-x86-hyperv-Update-struct-hv_enlightened_vmcs-definit.patch --]
[-- Type: text/x-patch, Size: 1558 bytes --]

From 377ec70ef19dc770bf0764e711408b89d53b36c6 Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Wed, 20 Apr 2022 14:42:50 +0200
Subject: [PATCH 2/4] x86/hyperv: Update 'struct hv_enlightened_vmcs'
 definition
Content-Type: text/plain

Updated Hyper-V Enlightened VMCS specification lists several new
fields for the following features:

- PerfGlobalCtrl
- EnclsExitingBitmap
- Tsc Scaling
- GuestLbrCtl
- CET
- SSP

Update the definition.

Note: The latest TLFS is available at
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/tlfs

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/include/asm/hyperv-tlfs.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 6f0acc45e67a..fd334e8defb7 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -559,9 +559,20 @@ struct hv_enlightened_vmcs {
 	u64 partition_assist_page;
 	u64 padding64_4[4];
 	u64 guest_bndcfgs;
-	u64 padding64_5[7];
+	u64 guest_ia32_perf_global_ctrl;
+	u64 guest_ia32_s_cet;
+	u64 guest_ssp;
+	u64 guest_ia32_int_ssp_table_addr;
+	u64 guest_ia32_lbr_ctl;
+	u64 padding64_5[2];
 	u64 xss_exit_bitmap;
-	u64 padding64_6[7];
+	u64 host_ia32_perf_global_ctrl;
+	u64 encls_exiting_bitmap;
+	u64 tsc_multiplier;
+	u64 host_ia32_s_cet;
+	u64 host_ssp;
+	u64 host_ia32_int_ssp_table_addr;
+	u64 padding64_6;
 } __packed;
 
 #define HV_VMX_ENLIGHTENED_CLEAN_FIELD_NONE			0
-- 
2.35.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-KVM-VMX-Define-VMCS-to-EVMCS-conversion-for-the-new-.patch --]
[-- Type: text/x-patch, Size: 3125 bytes --]

From 1c1be861161cb95f2b78727a6b7edda277ba036e Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Wed, 20 Apr 2022 15:41:01 +0200
Subject: [PATCH 3/4] KVM: VMX: Define VMCS-to-EVMCS conversion for the new
 fields
Content-Type: text/plain

Enlightened VMCS v1 definition was updated with new fields, support
them in KVM by defining VMCS-to-EVMCS conversion.

Note: SSP, CET and Guest LBR features are not supported by KVM yet and
the corresponding fields are not defined in 'enum vmcs_field', leave
them commented out for now.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/vmx/evmcs.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
index 87e3dc10edf4..61a702a804f8 100644
--- a/arch/x86/kvm/vmx/evmcs.c
+++ b/arch/x86/kvm/vmx/evmcs.c
@@ -28,6 +28,8 @@ const struct evmcs_field vmcs_field_to_evmcs_1[] = {
 		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
 	EVMCS1_FIELD(HOST_IA32_EFER, host_ia32_efer,
 		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
+	EVMCS1_FIELD(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl,
+		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
 	EVMCS1_FIELD(HOST_CR0, host_cr0,
 		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
 	EVMCS1_FIELD(HOST_CR3, host_cr3,
@@ -78,6 +80,8 @@ const struct evmcs_field vmcs_field_to_evmcs_1[] = {
 		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
 	EVMCS1_FIELD(GUEST_IA32_EFER, guest_ia32_efer,
 		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
+	EVMCS1_FIELD(GUEST_IA32_PERF_GLOBAL_CTRL, guest_ia32_perf_global_ctrl,
+		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
 	EVMCS1_FIELD(GUEST_PDPTR0, guest_pdptr0,
 		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
 	EVMCS1_FIELD(GUEST_PDPTR1, guest_pdptr1,
@@ -126,6 +130,28 @@ const struct evmcs_field vmcs_field_to_evmcs_1[] = {
 		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
 	EVMCS1_FIELD(XSS_EXIT_BITMAP, xss_exit_bitmap,
 		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_CONTROL_GRP2),
+	EVMCS1_FIELD(ENCLS_EXITING_BITMAP, encls_exiting_bitmap,
+		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_CONTROL_GRP2),
+	EVMCS1_FIELD(TSC_MULTIPLIER, tsc_multiplier,
+		     HV_VMX_ENLIGHTENED_CLEAN_FIELD_CONTROL_GRP2),
+	/*
+	 * Not used by KVM:
+	 *
+	 * EVMCS1_FIELD(0x00006828, guest_ia32_s_cet,
+	 *	     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
+	 * EVMCS1_FIELD(0x0000682A, guest_ssp,
+	 *	     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_BASIC),
+	 * EVMCS1_FIELD(0x0000682C, guest_ia32_int_ssp_table_addr,
+	 *	     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
+	 * EVMCS1_FIELD(0x00002816, guest_ia32_lbr_ctl,
+	 *	     HV_VMX_ENLIGHTENED_CLEAN_FIELD_GUEST_GRP1),
+	 * EVMCS1_FIELD(0x00006C18, host_ia32_s_cet,
+	 *	     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
+	 * EVMCS1_FIELD(0x00006C1A, host_ssp,
+	 *	     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
+	 * EVMCS1_FIELD(0x00006C1C, host_ia32_int_ssp_table_addr,
+	 *	     HV_VMX_ENLIGHTENED_CLEAN_FIELD_HOST_GRP1),
+	 */
 
 	/* 64 bit read only */
 	EVMCS1_FIELD(GUEST_PHYSICAL_ADDRESS, guest_physical_address,
-- 
2.35.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0004-KVM-VMX-Support-TSC-scaling-with-enlightened-VMCS.patch --]
[-- Type: text/x-patch, Size: 3317 bytes --]

From 5870058d2be9b8d2e34604e7f67eb7522f554dd9 Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: Wed, 15 Jun 2022 14:03:01 +0200
Subject: [PATCH 4/4] KVM: VMX: Support TSC scaling with enlightened VMCS
Content-Type: text/plain

Enlightened VMCS v1 now includes the required field for TSC scaling
feature so SECONDARY_EXEC_TSC_SCALING doesn't need to be filtered out
for KVM on Hyper-V case. Hyper-V on KVM is, however, trickier: to not
break live migration to older KVMs which may not support the feature
it needs to stay filtered out. Eventually, a new KVM capability indicating
support for the new fields will need to be introduced.

While on it, update the comment why VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL/
VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL are kept filtered out and add
missing spaces in trace_kvm_nested_vmenter_failed() strings making the
output ugly.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/vmx/evmcs.c |  8 +++++++-
 arch/x86/kvm/vmx/evmcs.h | 11 ++++-------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
index 61a702a804f8..6ed4bb2e676e 100644
--- a/arch/x86/kvm/vmx/evmcs.c
+++ b/arch/x86/kvm/vmx/evmcs.c
@@ -385,7 +385,13 @@ void nested_evmcs_filter_control_msr(u32 msr_index, u64 *pdata)
 		ctl_high &= ~EVMCS1_UNSUPPORTED_VMENTRY_CTRL;
 		break;
 	case MSR_IA32_VMX_PROCBASED_CTLS2:
-		ctl_high &= ~EVMCS1_UNSUPPORTED_2NDEXEC;
+		/*
+		 * Initially, SECONDARY_EXEC_TSC_SCALING was filtered out as there was no
+		 * TscMultiplier field defined in eVMCS. Keep the status quo to not break
+		 * live migration.
+		 */
+		ctl_high &= ~(EVMCS1_UNSUPPORTED_2NDEXEC |
+			      SECONDARY_EXEC_TSC_SCALING);
 		break;
 	case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
 	case MSR_IA32_VMX_PINBASED_CTLS:
diff --git a/arch/x86/kvm/vmx/evmcs.h b/arch/x86/kvm/vmx/evmcs.h
index 8d70f9aea94b..5fd9292be6bb 100644
--- a/arch/x86/kvm/vmx/evmcs.h
+++ b/arch/x86/kvm/vmx/evmcs.h
@@ -37,16 +37,14 @@ DECLARE_STATIC_KEY_FALSE(enable_evmcs);
  *	EPTP_LIST_ADDRESS               = 0x00002024,
  *	VMREAD_BITMAP                   = 0x00002026,
  *	VMWRITE_BITMAP                  = 0x00002028,
- *
- *	TSC_MULTIPLIER                  = 0x00002032,
  *	PLE_GAP                         = 0x00004020,
  *	PLE_WINDOW                      = 0x00004022,
  *	VMX_PREEMPTION_TIMER_VALUE      = 0x0000482E,
- *      GUEST_IA32_PERF_GLOBAL_CTRL     = 0x00002808,
- *      HOST_IA32_PERF_GLOBAL_CTRL      = 0x00002c04,
  *
- * Currently unsupported in KVM:
- *	GUEST_IA32_RTIT_CTL		= 0x00002814,
+ *	While GUEST_IA32_PERF_GLOBAL_CTRL and HOST_IA32_PERF_GLOBAL_CTRL
+ *	are present in eVMCSv1, Windows 11 still has issues booting when
+ *	VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL/VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL
+ *	are exposed to it, keep them filtered out.
  */
 #define EVMCS1_UNSUPPORTED_PINCTRL (PIN_BASED_POSTED_INTR | \
 				    PIN_BASED_VMX_PREEMPTION_TIMER)
@@ -57,7 +55,6 @@ DECLARE_STATIC_KEY_FALSE(enable_evmcs);
 	 SECONDARY_EXEC_ENABLE_PML |					\
 	 SECONDARY_EXEC_ENABLE_VMFUNC |					\
 	 SECONDARY_EXEC_SHADOW_VMCS |					\
-	 SECONDARY_EXEC_TSC_SCALING |					\
 	 SECONDARY_EXEC_PAUSE_LOOP_EXITING)
 #define EVMCS1_UNSUPPORTED_VMEXIT_CTRL					\
 	(VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL |				\
-- 
2.35.3

Re: [PATCH] KVM: nVMX: Don't expose TSC scaling to L1 when on Hyper-V

[Anirudh Rayabharam]

On Thu, Jun 23, 2022 at 01:49:30PM +0200, Vitaly Kuznetsov wrote:
> Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:
> 
> > On Wed, Jun 22, 2022 at 06:48:50PM +0200, Vitaly Kuznetsov wrote:
> >> Anirudh Rayabharam <anrayabh@linux.microsoft.com> writes:
> >> 
> >> > On Wed, Jun 22, 2022 at 04:35:27PM +0200, Vitaly Kuznetsov wrote:
> >> 
> >> ...
> >> 
> >> >> 
> >> >> I've tried to pick it up but it's actually much harder than I think. The
> >> >> patch has some minor issues ('&vmcs_config.nested' needs to be switched
> >> >> to '&vmcs_conf->nested' in nested_vmx_setup_ctls_msrs()), but the main
> >> >> problem is that the set of controls nested_vmx_setup_ctls_msrs() needs
> >> >> is NOT a subset of vmcs_config (setup_vmcs_config()). I was able to
> >> >> identify at least:
> >> 
> >> ...
> >> 
> >> I've jsut sent "[PATCH RFC v1 00/10] KVM: nVMX: Use vmcs_config for
> >> setting up nested VMX MSRs" which implements Sean's suggestion. Hope
> >> this is the way to go for mainline.
> >> 
> >> >
> >> > How about we do something simple like the patch below to start with?
> >> > This will easily apply to stable and we can continue improving upon
> >> > it with follow up patches on mainline.
> >> >
> >> 
> >> Personally, I'm not against this for @stable. Alternatively, in case the
> >
> > I think it's a good intermediate fix for mainline too. It is easier to land
> > it in stable if it already exists in mainline. It can stay in mainline
> > until your series lands and replaces it with the vmcs_config approach.
> >
> > What do you think?
> >
> 
> Paolo's call but personally I think both series can make 5.20 so there's
> no need for an intermediate solution.

Only reason I see for this intermediate solution is to automatically
land the fix in stable without bothering to write a special backport.

I will send it as a proper patch and see if there is any interest in
taking it.

	- Anirudh.