KVM: VMX: do not overwrite uptodate vcpu->arch.cr3 on KVM_SET_SREGS

KVM: VMX: do not overwrite uptodate vcpu->arch.cr3 on KVM_SET_SREGS

[Marcelo Tosatti]

Only decache guest CR3 value if vcpu->arch.cr3 is stale. 
Fixes loadvm with live guest.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Tested-by: Markus Schade <markus.schade@gmail.com>

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 750b0ff..772c4d2 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2049,7 +2049,9 @@ static void ept_update_paging_mode_cr0(unsigned long *hw_cr0,
 					unsigned long cr0,
 					struct kvm_vcpu *vcpu)
 {
-	vmx_decache_cr3(vcpu);
+
+	if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
+		vmx_decache_cr3(vcpu);
 	if (!(cr0 & X86_CR0_PG)) {
 		/* From paging/starting to nonpaging */
 		vmcs_write32(CPU_BASED_VM_EXEC_CONTROL,
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: KVM: VMX: do not overwrite uptodate vcpu->arch.cr3 on KVM_SET_SREGS

[Avi Kivity]

On 06/06/2011 08:27 PM, Marcelo Tosatti wrote:
> Only decache guest CR3 value if vcpu->arch.cr3 is stale.
> Fixes loadvm with live guest.
>
>
> @@ -2049,7 +2049,9 @@ static void ept_update_paging_mode_cr0(unsigned long *hw_cr0,
>   					unsigned long cr0,
>   					struct kvm_vcpu *vcpu)
>   {
> -	vmx_decache_cr3(vcpu);
> +
> +	if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
> +		vmx_decache_cr3(vcpu);
>   	if (!(cr0&  X86_CR0_PG)) {
>   		/* From paging/starting to nonpaging */
>   		vmcs_write32(CPU_BASED_VM_EXEC_CONTROL,

Applied and queued, but I think there is something rotten here.  How 
does arch.cr3 get into GUEST_CR3 after KVM_SET_SREGS?  arch.cr3 is a 
supposed to be write-through cache - it only has a bit in regs_avail, 
not regs_dirty.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: KVM: VMX: do not overwrite uptodate vcpu->arch.cr3 on KVM_SET_SREGS

[Marcelo Tosatti]

On Tue, Jun 07, 2011 at 11:24:49AM +0300, Avi Kivity wrote:
> On 06/06/2011 08:27 PM, Marcelo Tosatti wrote:
> >Only decache guest CR3 value if vcpu->arch.cr3 is stale.
> >Fixes loadvm with live guest.
> >
> >
> >@@ -2049,7 +2049,9 @@ static void ept_update_paging_mode_cr0(unsigned long *hw_cr0,
> >  					unsigned long cr0,
> >  					struct kvm_vcpu *vcpu)
> >  {
> >-	vmx_decache_cr3(vcpu);
> >+
> >+	if (!test_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail))
> >+		vmx_decache_cr3(vcpu);
> >  	if (!(cr0&  X86_CR0_PG)) {
> >  		/* From paging/starting to nonpaging */
> >  		vmcs_write32(CPU_BASED_VM_EXEC_CONTROL,
> 
> Applied and queued, but I think there is something rotten here.  How
> does arch.cr3 get into GUEST_CR3 after KVM_SET_SREGS?  arch.cr3 is a
> supposed to be write-through cache - it only has a bit in
> regs_avail, not regs_dirty.


KVM_SET_SREGS sets good kvm->arch.cr3. Then vmx_decache_cr3 overwrites
kvm->arch.cr3 from GUEST_CR3, which is stale.

Re: KVM: VMX: do not overwrite uptodate vcpu->arch.cr3 on KVM_SET_SREGS

[Avi Kivity]

On 06/09/2011 08:11 PM, Marcelo Tosatti wrote:
> >
> >  Applied and queued, but I think there is something rotten here.  How
> >  does arch.cr3 get into GUEST_CR3 after KVM_SET_SREGS?  arch.cr3 is a
> >  supposed to be write-through cache - it only has a bit in
> >  regs_avail, not regs_dirty.
>
>
> KVM_SET_SREGS sets good kvm->arch.cr3. Then vmx_decache_cr3 overwrites
> kvm->arch.cr3 from GUEST_CR3, which is stale.
>

Right.  Should have called kvm_set_cr3().  It's tricky since we need to 
set the order of writing various registers, I'll take a look.

-- 
error compiling committee.c: too many arguments to function