Re: [kvm-devel] [PATCH 02/33] KVM: MMU: emulated cmpxchg8b should be atomic on i386

[Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>]

On Sun, Jan 06, 2008 at 04:39:08PM +0200, Avi Kivity wrote:
> From: Marcelo Tosatti <marcelo@kvack.org>
> 
> Emulate cmpxchg8b atomically on i386. This is required to avoid a guest
> pte walker from seeing a splitted write.
> 
> [avi: make it compile]
> 
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> Signed-off-by: Avi Kivity <avi@qumranet.com>
> ---

This one doesn't look like the one in the current tree, and avoids the reuse
of addr by moving the call to get the gpa up instead of renaming the variable
as propossed in :

  http://article.gmane.org/gmane.comp.emulators.kvm.devel/11511

IMHO renaming the parameter variable is probably better in the long run, as it
could result otherwise into scope related bugs like the one from kvm-59

> diff --git a/drivers/kvm/x86.c b/drivers/kvm/x86.c
> index 9db4e32..3b79684 100644
> --- a/drivers/kvm/x86.c
> +++ b/drivers/kvm/x86.c
> @@ -1674,6 +1675,31 @@ static int emulator_cmpxchg_emulated(unsigned long addr,
>  		reported = 1;
>  		printk(KERN_WARNING "kvm: emulating exchange as write\n");
>  	}
> +#ifndef CONFIG_X86_64
> +	/* guests cmpxchg8b have to be emulated atomically */
> +	if (bytes == 8) {
> +		gpa_t gpa = vcpu->mmu.gva_to_gpa(vcpu, addr);
> +		struct page *page;
> +		char *addr;
> +		u64 val;
> +
> +		if (gpa == UNMAPPED_GVA ||
> +		   (gpa & PAGE_MASK) == APIC_DEFAULT_PHYS_BASE)
> +			goto emul_write;
> +
> +		if (((gpa + bytes - 1) & PAGE_MASK) != (gpa & PAGE_MASK))
> +			goto emul_write;
> +
> +		val = *(u64 *)new;
> +		page = gfn_to_page(vcpu->kvm, gpa >> PAGE_SHIFT);
> +		addr = kmap_atomic(page, KM_USER0);
> +		set_64bit((u64 *)(addr + offset_in_page(gpa)), val);
> +		kunmap_atomic(addr, KM_USER0);
> +		kvm_release_page_dirty(page);
> +	}
> +emul_write:
> +#endif
> +
>  	return emulator_write_emulated(addr, new, bytes, vcpu);
>  }
>  
> -- 
> 1.5.3.7