linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: SVM: Clear the CR4 register on reset
@ 2021-03-02 18:51 Babu Moger
  2021-03-02 19:20 ` Sean Christopherson
  2021-03-02 19:39 ` Paolo Bonzini
  0 siblings, 2 replies; 4+ messages in thread
From: Babu Moger @ 2021-03-02 18:51 UTC (permalink / raw)
  To: pbonzini
  Cc: wanpengli, kvm, seanjc, joro, x86, linux-kernel, babu.moger,
	mingo, bp, hpa, vkuznets, tglx, jmattson

This problem was reported on a SVM guest while executing kexec.
Kexec fails to load the new kernel when the PCID feature is enabled.

When kexec starts loading the new kernel, it starts the process by
resetting the vCPU's and then bringing each vCPU online one by one.
The vCPU reset is supposed to reset all the register states before the
vCPUs are brought online. However, the CR4 register is not reset during
this process. If this register is already setup during the last boot,
all the flags can remain intact. The X86_CR4_PCIDE bit can only be
enabled in long mode. So, it must be enabled much later in SMP
initialization.  Having the X86_CR4_PCIDE bit set during SMP boot can
cause a boot failures.

Fix the issue by resetting the CR4 register in init_vmcb().

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kvm/svm/svm.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index c636021b066b..baee91c1e936 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1200,6 +1200,7 @@ static void init_vmcb(struct vcpu_svm *svm)
 	init_sys_seg(&save->ldtr, SEG_TYPE_LDT);
 	init_sys_seg(&save->tr, SEG_TYPE_BUSY_TSS16);
 
+	svm_set_cr4(&svm->vcpu, 0);
 	svm_set_efer(&svm->vcpu, 0);
 	save->dr6 = 0xffff0ff0;
 	kvm_set_rflags(&svm->vcpu, X86_EFLAGS_FIXED);


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: SVM: Clear the CR4 register on reset
  2021-03-02 18:51 [PATCH] KVM: SVM: Clear the CR4 register on reset Babu Moger
@ 2021-03-02 19:20 ` Sean Christopherson
  2021-03-02 19:29   ` Babu Moger
  2021-03-02 19:39 ` Paolo Bonzini
  1 sibling, 1 reply; 4+ messages in thread
From: Sean Christopherson @ 2021-03-02 19:20 UTC (permalink / raw)
  To: Babu Moger
  Cc: pbonzini, wanpengli, kvm, joro, x86, linux-kernel, mingo, bp,
	hpa, vkuznets, tglx, jmattson

On Tue, Mar 02, 2021, Babu Moger wrote:
> This problem was reported on a SVM guest while executing kexec.
> Kexec fails to load the new kernel when the PCID feature is enabled.
> 
> When kexec starts loading the new kernel, it starts the process by
> resetting the vCPU's and then bringing each vCPU online one by one.
> The vCPU reset is supposed to reset all the register states before the
> vCPUs are brought online. However, the CR4 register is not reset during
> this process. If this register is already setup during the last boot,
> all the flags can remain intact. The X86_CR4_PCIDE bit can only be
> enabled in long mode. So, it must be enabled much later in SMP
> initialization.  Having the X86_CR4_PCIDE bit set during SMP boot can
> cause a boot failures.
> 
> Fix the issue by resetting the CR4 register in init_vmcb().
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>

Cc: stable@vger.kernel.org

The bug goes back too far to have a meaningful Fixes.

Reviewed-by: Sean Christopherson <seanjc@google.com>


On a related topic, I think we can clean up the RESET/INIT flows by hoisting the
common code into kvm_vcpu_reset().  That would also provide good motivation for
removing the init_vmcb() call in svm_create_vcpu(), which is fully redundant
with the call in svm_vcpu_reset().  I'll put that on the todo list.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: SVM: Clear the CR4 register on reset
  2021-03-02 19:20 ` Sean Christopherson
@ 2021-03-02 19:29   ` Babu Moger
  0 siblings, 0 replies; 4+ messages in thread
From: Babu Moger @ 2021-03-02 19:29 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: pbonzini, wanpengli, kvm, joro, x86, linux-kernel, mingo, bp,
	hpa, vkuznets, tglx, jmattson



On 3/2/21 1:20 PM, Sean Christopherson wrote:
> On Tue, Mar 02, 2021, Babu Moger wrote:
>> This problem was reported on a SVM guest while executing kexec.
>> Kexec fails to load the new kernel when the PCID feature is enabled.
>>
>> When kexec starts loading the new kernel, it starts the process by
>> resetting the vCPU's and then bringing each vCPU online one by one.
>> The vCPU reset is supposed to reset all the register states before the
>> vCPUs are brought online. However, the CR4 register is not reset during
>> this process. If this register is already setup during the last boot,
>> all the flags can remain intact. The X86_CR4_PCIDE bit can only be
>> enabled in long mode. So, it must be enabled much later in SMP
>> initialization.  Having the X86_CR4_PCIDE bit set during SMP boot can
>> cause a boot failures.
>>
>> Fix the issue by resetting the CR4 register in init_vmcb().
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
> 
> Cc: stable@vger.kernel.org
> 
> The bug goes back too far to have a meaningful Fixes.
> 
> Reviewed-by: Sean Christopherson <seanjc@google.com>

Sean, Thanks
> 
> 
> On a related topic, I think we can clean up the RESET/INIT flows by hoisting the
> common code into kvm_vcpu_reset().  That would also provide good motivation for
> removing the init_vmcb() call in svm_create_vcpu(), which is fully redundant
> with the call in svm_vcpu_reset().  I'll put that on the todo list.

Yes.Please.Thought about cleaning init_vmcb and svm_vcpu_reset little bit.
That will require some more tests and review. We didn't want to delay the
fix for that now.
Thanks
Babu

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] KVM: SVM: Clear the CR4 register on reset
  2021-03-02 18:51 [PATCH] KVM: SVM: Clear the CR4 register on reset Babu Moger
  2021-03-02 19:20 ` Sean Christopherson
@ 2021-03-02 19:39 ` Paolo Bonzini
  1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2021-03-02 19:39 UTC (permalink / raw)
  To: Babu Moger
  Cc: wanpengli, kvm, seanjc, joro, x86, linux-kernel, mingo, bp, hpa,
	vkuznets, tglx, jmattson

On 02/03/21 19:51, Babu Moger wrote:
> This problem was reported on a SVM guest while executing kexec.
> Kexec fails to load the new kernel when the PCID feature is enabled.
> 
> When kexec starts loading the new kernel, it starts the process by
> resetting the vCPU's and then bringing each vCPU online one by one.
> The vCPU reset is supposed to reset all the register states before the
> vCPUs are brought online. However, the CR4 register is not reset during
> this process. If this register is already setup during the last boot,
> all the flags can remain intact. The X86_CR4_PCIDE bit can only be
> enabled in long mode. So, it must be enabled much later in SMP
> initialization.  Having the X86_CR4_PCIDE bit set during SMP boot can
> cause a boot failures.
> 
> Fix the issue by resetting the CR4 register in init_vmcb().
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>   arch/x86/kvm/svm/svm.c |    1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index c636021b066b..baee91c1e936 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1200,6 +1200,7 @@ static void init_vmcb(struct vcpu_svm *svm)
>   	init_sys_seg(&save->ldtr, SEG_TYPE_LDT);
>   	init_sys_seg(&save->tr, SEG_TYPE_BUSY_TSS16);
>   
> +	svm_set_cr4(&svm->vcpu, 0);
>   	svm_set_efer(&svm->vcpu, 0);
>   	save->dr6 = 0xffff0ff0;
>   	kvm_set_rflags(&svm->vcpu, X86_EFLAGS_FIXED);
> 

Queued, thanks.

Paolo


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-02 22:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 18:51 [PATCH] KVM: SVM: Clear the CR4 register on reset Babu Moger
2021-03-02 19:20 ` Sean Christopherson
2021-03-02 19:29   ` Babu Moger
2021-03-02 19:39 ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).