Re: [PATCH v2] KVM: VMX: Do not overwrite vcpu->srcu_idx in vmx_vcpu_reset

[Jan Kiszka <jan.kiszka@web.de>]

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

On 2013-03-14 20:39, Jan Kiszka wrote:
> vmx_vcpu_reset may now be called while already holding the srcu lock, so
> we may overwrite what was already saved there. Save and restore it.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Even if this should be unneeded, it looks more consistent. In any case,
> all versions on the table, pick what you prefer.
> 
>  arch/x86/kvm/vmx.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 958ac3a..7bc49ca 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -4117,6 +4117,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
>  	u64 msr;
> +	int idx;
>  
>  	vmx->rmode.vm86_active = 0;
>  
> @@ -4190,9 +4191,11 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu)
>  		vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
>  
>  	vmx->vcpu.arch.cr0 = X86_CR0_NW | X86_CR0_CD | X86_CR0_ET;
> +	idx = vcpu->srcu_idx;
>  	vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
>  	vmx_set_cr0(&vmx->vcpu, kvm_read_cr0(vcpu)); /* enter rmode */
>  	srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
> +	vcpu->srcu_idx = idx;
>  	vmx_set_cr4(&vmx->vcpu, 0);
>  	vmx_set_efer(&vmx->vcpu, 0);
>  	vmx_fpu_activate(&vmx->vcpu);
> 

This cannot work either: I think we really need to drop the srcu lock
before calling vmx_set_tss_addr. But if we nest the lock, we may only
drop it once now in enter_rmode.

OK, I'll propose a patch to remove that TSS bug workaround from
enter_rmode. Will dig a bit in the archives as well to check which
version of qemu-kvm was actually exposing this.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]