From: Paolo Bonzini <[email protected]>
[ Upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ]
If the guest is configured to have SPEC_CTRL but the host does not
(which is a nonsensical configuration but these are not explicitly
forbidden) then a host-initiated MSR write can write vmx->spec_ctrl
(respectively svm->spec_ctrl) and trigger a #GP when KVM tries to
restore the host value of the MSR. Add a more comprehensive check
for valid bits of SPEC_CTRL, covering host CPUID flags and,
since we are at it and it is more correct that way, guest CPUID
flags too.
For AMD, remove the unnecessary is_guest_mode check around setting
the MSR interception bitmap, so that the code looks the same as
for Intel.
Cc: Jim Mattson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
arch/x86/kvm/svm.c | 9 +++------
arch/x86/kvm/vmx/vmx.c | 7 +++----
arch/x86/kvm/x86.c | 22 ++++++++++++++++++++++
arch/x86/kvm/x86.h | 1 +
4 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c79c1a07f44b9..72bf1d8175ac2 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4322,12 +4322,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
!guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
return 1;
- /* The STIBP bit doesn't fault even if it's not advertised */
- if (data & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD))
+ if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
return 1;
svm->spec_ctrl = data;
-
if (!data)
break;
@@ -4351,13 +4349,12 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
if (data & ~PRED_CMD_IBPB)
return 1;
-
+ if (!boot_cpu_has(X86_FEATURE_AMD_IBPB))
+ return 1;
if (!data)
break;
wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB);
- if (is_guest_mode(vcpu))
- break;
set_msr_interception(svm->msrpm, MSR_IA32_PRED_CMD, 0, 1);
break;
case MSR_AMD64_VIRT_SPEC_CTRL:
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 2a1ed3aae100e..8450fce70bd96 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1974,12 +1974,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
return 1;
- /* The STIBP bit doesn't fault even if it's not advertised */
- if (data & ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD))
+ if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
return 1;
vmx->spec_ctrl = data;
-
if (!data)
break;
@@ -2006,7 +2004,8 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
if (data & ~PRED_CMD_IBPB)
return 1;
-
+ if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL))
+ return 1;
if (!data)
break;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b7f86acb8c911..72990c3c6faf7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10369,6 +10369,28 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvm_arch_no_poll);
+u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu)
+{
+ uint64_t bits = SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD;
+
+ /* The STIBP bit doesn't fault even if it's not advertised */
+ if (!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS))
+ bits &= ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP);
+ if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
+ !boot_cpu_has(X86_FEATURE_AMD_IBRS))
+ bits &= ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP);
+
+ if (!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL_SSBD) &&
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
+ bits &= ~SPEC_CTRL_SSBD;
+ if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) &&
+ !boot_cpu_has(X86_FEATURE_AMD_SSBD))
+ bits &= ~SPEC_CTRL_SSBD;
+
+ return bits;
+}
+EXPORT_SYMBOL_GPL(kvm_spec_ctrl_valid_bits);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_exit);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_fast_mmio);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index de6b55484876a..301286d924320 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -368,5 +368,6 @@ static inline bool kvm_pat_valid(u64 data)
void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu);
void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu);
+u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu);
#endif
--
2.27.0
On 04.01.21 16:57, Greg Kroah-Hartman wrote:
> From: Paolo Bonzini <[email protected]>
>
> [ Upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ]
>
> If the guest is configured to have SPEC_CTRL but the host does not
> (which is a nonsensical configuration but these are not explicitly
> forbidden) then a host-initiated MSR write can write vmx->spec_ctrl
> (respectively svm->spec_ctrl) and trigger a #GP when KVM tries to
> restore the host value of the MSR. Add a more comprehensive check
> for valid bits of SPEC_CTRL, covering host CPUID flags and,
> since we are at it and it is more correct that way, guest CPUID
> flags too.
>
> For AMD, remove the unnecessary is_guest_mode check around setting
> the MSR interception bitmap, so that the code looks the same as
> for Intel.
>
A git bisect between 5.4.86 and 5.4.98 showed that this breaks boot of QEMU
guests running Windows 10 20H2 on AMD Ryzen X3700 CPUs with a BSOD showing
"KERNEL SECURITY CHECK FAILURE".
Reverting this commit or setting the CPU type of the QEMU/KVM command from
host to qemu64 allows one to boot Windows 10 in the VM again.
I found a followup, commit 841c2be09fe4f495fe5224952a419bd8c7e5b455 [0],
which has a fixes line for this commit and mentions Zen2 AMD CPUs (which
the X3700 is).
Applying a backport of that commit on top of 5.4.98 stable tree fixed the
issue here see below for the backport I used, it applies also cleanly on the
more current 5.4.101 release.
So can you please add this patch to the stable trees that backported the
problematic upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ?
If I should submit this in any other way just ask, was not sure about
what works best with a patch which cannot be cherry-picked cleanly.
cheers,
Thomas
[0]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=841c2be09fe4f495fe5224952a419bd8c7e5b455
----8<---
From: Maxim Levitsky <[email protected]>
Date: Wed, 8 Jul 2020 14:57:31 +0300
Subject: [PATCH] kvm: x86: replace kvm_spec_ctrl_test_value with runtime test
on the host
To avoid complex and in some cases incorrect logic in
kvm_spec_ctrl_test_value, just try the guest's given value on the host
processor instead, and if it doesn't #GP, allow the guest to set it.
One such case is when host CPU supports STIBP mitigation
but doesn't support IBRS (as is the case with some Zen2 AMD cpus),
and in this case we were giving guest #GP when it tried to use STIBP
The reason why can can do the host test is that IA32_SPEC_CTRL msr is
passed to the guest, after the guest sets it to a non zero value
for the first time (due to performance reasons),
and as as result of this, it is pointless to emulate #GP condition on
this first access, in a different way than what the host CPU does.
This is based on a patch from Sean Christopherson, who suggested this idea.
Fixes: 6441fa6178f5 ("KVM: x86: avoid incorrect writes to host MSR_IA32_SPEC_CTRL")
Cc: [email protected]
Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Maxim Levitsky <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
(cherry picked from commit 841c2be09fe4f495fe5224952a419bd8c7e5b455)
[ Thomas: resolved merge conflict in arch/x86/kvm/x86.h and
replicated the change to arch/x86/kvm/svm/svm.c to the in 5.4 not
yet moved out arch/x86/kvm/svm.c ]
Signed-off-by: Thomas Lamprecht <[email protected]>
---
arch/x86/kvm/svm.c | 2 +-
arch/x86/kvm/vmx/vmx.c | 2 +-
arch/x86/kvm/x86.c | 38 +++++++++++++++++++++-----------------
arch/x86/kvm/x86.h | 2 +-
4 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 8d386efc2540..372a958bec72 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4327,7 +4327,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
!guest_has_spec_ctrl_msr(vcpu))
return 1;
- if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
+ if (kvm_spec_ctrl_test_value(data))
return 1;
svm->spec_ctrl = data;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e7fd2f00edc1..e177848a3631 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1974,7 +1974,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
!guest_has_spec_ctrl_msr(vcpu))
return 1;
- if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
+ if (kvm_spec_ctrl_test_value(data))
return 1;
vmx->spec_ctrl = data;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f5a827150664..1330fc4dc7c5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10376,28 +10376,32 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvm_arch_no_poll);
-u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu)
+
+int kvm_spec_ctrl_test_value(u64 value)
{
- uint64_t bits = SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD;
+ /*
+ * test that setting IA32_SPEC_CTRL to given value
+ * is allowed by the host processor
+ */
- /* The STIBP bit doesn't fault even if it's not advertised */
- if (!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
- !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS))
- bits &= ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP);
- if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
- !boot_cpu_has(X86_FEATURE_AMD_IBRS))
- bits &= ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP);
+ u64 saved_value;
+ unsigned long flags;
+ int ret = 0;
- if (!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL_SSBD) &&
- !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
- bits &= ~SPEC_CTRL_SSBD;
- if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) &&
- !boot_cpu_has(X86_FEATURE_AMD_SSBD))
- bits &= ~SPEC_CTRL_SSBD;
+ local_irq_save(flags);
- return bits;
+ if (rdmsrl_safe(MSR_IA32_SPEC_CTRL, &saved_value))
+ ret = 1;
+ else if (wrmsrl_safe(MSR_IA32_SPEC_CTRL, value))
+ ret = 1;
+ else
+ wrmsrl(MSR_IA32_SPEC_CTRL, saved_value);
+
+ local_irq_restore(flags);
+
+ return ret;
}
-EXPORT_SYMBOL_GPL(kvm_spec_ctrl_valid_bits);
+EXPORT_SYMBOL_GPL(kvm_spec_ctrl_test_value);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_exit);
EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_fast_mmio);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 301286d92432..c520d373790a 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -368,6 +368,6 @@ static inline bool kvm_pat_valid(u64 data)
void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu);
void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu);
-u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu);
+int kvm_spec_ctrl_test_value(u64 value);
#endif
--
2.20.1
On 26/02/21 12:03, Thomas Lamprecht wrote:
> On 04.01.21 16:57, Greg Kroah-Hartman wrote:
>> From: Paolo Bonzini <[email protected]>
>>
>> [ Upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ]
>>
>> If the guest is configured to have SPEC_CTRL but the host does not
>> (which is a nonsensical configuration but these are not explicitly
>> forbidden) then a host-initiated MSR write can write vmx->spec_ctrl
>> (respectively svm->spec_ctrl) and trigger a #GP when KVM tries to
>> restore the host value of the MSR. Add a more comprehensive check
>> for valid bits of SPEC_CTRL, covering host CPUID flags and,
>> since we are at it and it is more correct that way, guest CPUID
>> flags too.
>>
>> For AMD, remove the unnecessary is_guest_mode check around setting
>> the MSR interception bitmap, so that the code looks the same as
>> for Intel.
>>
>
> A git bisect between 5.4.86 and 5.4.98 showed that this breaks boot of QEMU
> guests running Windows 10 20H2 on AMD Ryzen X3700 CPUs with a BSOD showing
> "KERNEL SECURITY CHECK FAILURE".
>
> Reverting this commit or setting the CPU type of the QEMU/KVM command from
> host to qemu64 allows one to boot Windows 10 in the VM again.
>
> I found a followup, commit 841c2be09fe4f495fe5224952a419bd8c7e5b455 [0],
> which has a fixes line for this commit and mentions Zen2 AMD CPUs (which
> the X3700 is).
> Applying a backport of that commit on top of 5.4.98 stable tree fixed the
> issue here see below for the backport I used, it applies also cleanly on the
> more current 5.4.101 release.
>
> So can you please add this patch to the stable trees that backported the
> problematic upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ?
>
> If I should submit this in any other way just ask, was not sure about
> what works best with a patch which cannot be cherry-picked cleanly.
Ok, I'll submit it.
Thanks for the testing.
Paolo
>
> cheers,
> Thomas
>
> [0]: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=841c2be09fe4f495fe5224952a419bd8c7e5b455
>
> ----8<---
> From: Maxim Levitsky <[email protected]>
> Date: Wed, 8 Jul 2020 14:57:31 +0300
> Subject: [PATCH] kvm: x86: replace kvm_spec_ctrl_test_value with runtime test
> on the host
>
> To avoid complex and in some cases incorrect logic in
> kvm_spec_ctrl_test_value, just try the guest's given value on the host
> processor instead, and if it doesn't #GP, allow the guest to set it.
>
> One such case is when host CPU supports STIBP mitigation
> but doesn't support IBRS (as is the case with some Zen2 AMD cpus),
> and in this case we were giving guest #GP when it tried to use STIBP
>
> The reason why can can do the host test is that IA32_SPEC_CTRL msr is
> passed to the guest, after the guest sets it to a non zero value
> for the first time (due to performance reasons),
> and as as result of this, it is pointless to emulate #GP condition on
> this first access, in a different way than what the host CPU does.
>
> This is based on a patch from Sean Christopherson, who suggested this idea.
>
> Fixes: 6441fa6178f5 ("KVM: x86: avoid incorrect writes to host MSR_IA32_SPEC_CTRL")
> Cc: [email protected]
> Suggested-by: Sean Christopherson <[email protected]>
> Signed-off-by: Maxim Levitsky <[email protected]>
> Message-Id: <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> (cherry picked from commit 841c2be09fe4f495fe5224952a419bd8c7e5b455)
> [ Thomas: resolved merge conflict in arch/x86/kvm/x86.h and
> replicated the change to arch/x86/kvm/svm/svm.c to the in 5.4 not
> yet moved out arch/x86/kvm/svm.c ]
> Signed-off-by: Thomas Lamprecht <[email protected]>
> ---
> arch/x86/kvm/svm.c | 2 +-
> arch/x86/kvm/vmx/vmx.c | 2 +-
> arch/x86/kvm/x86.c | 38 +++++++++++++++++++++-----------------
> arch/x86/kvm/x86.h | 2 +-
> 4 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 8d386efc2540..372a958bec72 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4327,7 +4327,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
> !guest_has_spec_ctrl_msr(vcpu))
> return 1;
>
> - if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
> + if (kvm_spec_ctrl_test_value(data))
> return 1;
>
> svm->spec_ctrl = data;
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index e7fd2f00edc1..e177848a3631 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1974,7 +1974,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> !guest_has_spec_ctrl_msr(vcpu))
> return 1;
>
> - if (data & ~kvm_spec_ctrl_valid_bits(vcpu))
> + if (kvm_spec_ctrl_test_value(data))
> return 1;
>
> vmx->spec_ctrl = data;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f5a827150664..1330fc4dc7c5 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10376,28 +10376,32 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
> }
> EXPORT_SYMBOL_GPL(kvm_arch_no_poll);
>
> -u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu)
> +
> +int kvm_spec_ctrl_test_value(u64 value)
> {
> - uint64_t bits = SPEC_CTRL_IBRS | SPEC_CTRL_STIBP | SPEC_CTRL_SSBD;
> + /*
> + * test that setting IA32_SPEC_CTRL to given value
> + * is allowed by the host processor
> + */
>
> - /* The STIBP bit doesn't fault even if it's not advertised */
> - if (!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
> - !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS))
> - bits &= ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP);
> - if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL) &&
> - !boot_cpu_has(X86_FEATURE_AMD_IBRS))
> - bits &= ~(SPEC_CTRL_IBRS | SPEC_CTRL_STIBP);
> + u64 saved_value;
> + unsigned long flags;
> + int ret = 0;
>
> - if (!guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL_SSBD) &&
> - !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
> - bits &= ~SPEC_CTRL_SSBD;
> - if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) &&
> - !boot_cpu_has(X86_FEATURE_AMD_SSBD))
> - bits &= ~SPEC_CTRL_SSBD;
> + local_irq_save(flags);
>
> - return bits;
> + if (rdmsrl_safe(MSR_IA32_SPEC_CTRL, &saved_value))
> + ret = 1;
> + else if (wrmsrl_safe(MSR_IA32_SPEC_CTRL, value))
> + ret = 1;
> + else
> + wrmsrl(MSR_IA32_SPEC_CTRL, saved_value);
> +
> + local_irq_restore(flags);
> +
> + return ret;
> }
> -EXPORT_SYMBOL_GPL(kvm_spec_ctrl_valid_bits);
> +EXPORT_SYMBOL_GPL(kvm_spec_ctrl_test_value);
>
> EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_exit);
> EXPORT_TRACEPOINT_SYMBOL_GPL(kvm_fast_mmio);
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index 301286d92432..c520d373790a 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -368,6 +368,6 @@ static inline bool kvm_pat_valid(u64 data)
>
> void kvm_load_guest_xcr0(struct kvm_vcpu *vcpu);
> void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu);
> -u64 kvm_spec_ctrl_valid_bits(struct kvm_vcpu *vcpu);
> +int kvm_spec_ctrl_test_value(u64 value);
>
> #endif
>
On Fri, Feb 26, 2021 at 12:27:49PM +0100, Paolo Bonzini wrote:
> On 26/02/21 12:03, Thomas Lamprecht wrote:
> > On 04.01.21 16:57, Greg Kroah-Hartman wrote:
> > > From: Paolo Bonzini <[email protected]>
> > >
> > > [ Upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ]
> > >
> > > If the guest is configured to have SPEC_CTRL but the host does not
> > > (which is a nonsensical configuration but these are not explicitly
> > > forbidden) then a host-initiated MSR write can write vmx->spec_ctrl
> > > (respectively svm->spec_ctrl) and trigger a #GP when KVM tries to
> > > restore the host value of the MSR. Add a more comprehensive check
> > > for valid bits of SPEC_CTRL, covering host CPUID flags and,
> > > since we are at it and it is more correct that way, guest CPUID
> > > flags too.
> > >
> > > For AMD, remove the unnecessary is_guest_mode check around setting
> > > the MSR interception bitmap, so that the code looks the same as
> > > for Intel.
> > >
> >
> > A git bisect between 5.4.86 and 5.4.98 showed that this breaks boot of QEMU
> > guests running Windows 10 20H2 on AMD Ryzen X3700 CPUs with a BSOD showing
> > "KERNEL SECURITY CHECK FAILURE".
> >
> > Reverting this commit or setting the CPU type of the QEMU/KVM command from
> > host to qemu64 allows one to boot Windows 10 in the VM again.
> >
> > I found a followup, commit 841c2be09fe4f495fe5224952a419bd8c7e5b455 [0],
> > which has a fixes line for this commit and mentions Zen2 AMD CPUs (which
> > the X3700 is).
> > Applying a backport of that commit on top of 5.4.98 stable tree fixed the
> > issue here see below for the backport I used, it applies also cleanly on the
> > more current 5.4.101 release.
> >
> > So can you please add this patch to the stable trees that backported the
> > problematic upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ?
> >
> > If I should submit this in any other way just ask, was not sure about
> > what works best with a patch which cannot be cherry-picked cleanly.
>
> Ok, I'll submit it.
>
> Thanks for the testing.
Does that mean I should not take the patch here in this email and that
you will submit it after some timeperiod, or that I should take this
patch as-is?
thanks,
greg k-h
On 26/02/21 13:59, Greg Kroah-Hartman wrote:
>>> So can you please add this patch to the stable trees that backported the
>>> problematic upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ?
>>>
>>> If I should submit this in any other way just ask, was not sure about
>>> what works best with a patch which cannot be cherry-picked cleanly.
>>
>> Ok, I'll submit it.
>>
>> Thanks for the testing.
>
> Does that mean I should not take the patch here in this email and that
> you will submit it after some timeperiod, or that I should take this
> patch as-is?
The patch that Thomas requested (commit 841c2be09fe) does not apply
cleanly, so I'll take care of sending the backport.
Paolo
On 26.02.21 15:15, Paolo Bonzini wrote:
> On 26/02/21 13:59, Greg Kroah-Hartman wrote:
>>>> So can you please add this patch to the stable trees that backported the
>>>> problematic upstream commit 6441fa6178f5456d1d4b512c08798888f99db185 ?
>>>>
>>>> If I should submit this in any other way just ask, was not sure about
>>>> what works best with a patch which cannot be cherry-picked cleanly.
>>>
>>> Ok, I'll submit it.
>>>
>>> Thanks for the testing.
>>
>> Does that mean I should not take the patch here in this email and that
>> you will submit it after some timeperiod, or that I should take this
>> patch as-is?
>
> The patch that Thomas requested (commit 841c2be09fe) does not apply cleanly, so I'll take care of sending the backport.
>
Note that the patch I added inline in my initial mail here was already
adapted to apply cleanly, at least on stable-5.4.y
May not have made that clear enough, so mentioning it here - ignore me this
message if that was read and thought of.
cheers,
Thomas
On 26/02/21 15:18, Thomas Lamprecht wrote:
>>> Does that mean I should not take the patch here in this email and that
>>> you will submit it after some timeperiod, or that I should take this
>>> patch as-is?
>> The patch that Thomas requested (commit 841c2be09fe) does not apply cleanly, so I'll take care of sending the backport.
>>
> Note that the patch I added inline in my initial mail here was already
> adapted to apply cleanly, at least on stable-5.4.y
>
> May not have made that clear enough, so mentioning it here - ignore me this
> message if that was read and thought of.
No, I just didn't notice at all that you had taken care of backporting
the patch. My brain processed it as the quote of the broken patch you
were replying to... I wear glasses after all.
Paolo