Marcelo Tosatti wrote:
> On Fri, Dec 07, 2007 at 07:06:26AM +0200, Avi Kivity wrote:
>
>> Marcelo Tosatti wrote:
>>
>>> Right, patch at end of the message restarts the process if the pte
>>> changes under the walker. The goto is pretty ugly, but I fail to see any
>>> elegant way of doing that. Ideas?
>>>
>>>
>>>
>> goto is fine for that. But there's a subtle livelock here: suppose vcpu
>> 0 is in guest mode with continuously updating a memory location. vcpu 1
>> is faulting with that memory location acting as a pte. While we're in
>> kernel mode, we aren't responding to signals like we should; so we need
>> to abort the walk and let the guest retry; that way we go through the
>> signal_pending() check.
>>
>> However, this is an intrusive change, so let's start with the goto and
>> drop it later in favor or an abort.
>>
>>
>>>>> @@ -1510,6 +1510,9 @@ static int emulator_write_phys(struct kvm_vcpu
>>>>> *vcpu, gpa_t gpa,
>>>>> {
>>>>> int ret;
>>>>>
>>>>> + /* No need for kvm_cmpxchg_guest_pte here, its the guest
>>>>> + * responsability to synchronize pte updates and page faults.
>>>>> + */
>>>>> ret = kvm_write_guest(vcpu->kvm, gpa, val, bytes);
>>>>> if (ret < 0)
>>>>> return 0;
>>>>>
>>>>>
>>>> Hmm. What if an i386 pae guest carefully uses cmpxchg8b to atomically
>>>> set a pte? kvm_write_guest() doesn't guarantee atomicity, so an
>>>> intended atomic write can be seen splitted by the guest walker doing a
>>>> concurrent walk.
>>>>
>>>>
>>> True, an atomic write is needed... a separate patch for that seems more
>>> appropriate.
>>>
>>>
>>>
>>>
>> Yes.
>>
>
> Hi Avi,
>
> How about the following
>
> diff --git a/drivers/kvm/x86.c b/drivers/kvm/x86.c
> index c70ac33..8678651 100644
> --- a/drivers/kvm/x86.c
> +++ b/drivers/kvm/x86.c
> @@ -25,6 +25,7 @@
> #include <linux/vmalloc.h>
> #include <linux/module.h>
> #include <linux/mman.h>
> +#include <linux/highmem.h>
>
> #include <asm/uaccess.h>
> #include <asm/msr.h>
> @@ -1589,6 +1590,34 @@ static int emulator_cmpxchg_emulated(unsigned long
> addr,
> reported = 1;
> printk(KERN_WARNING "kvm: emulating exchange as write\n");
> }
> +
> + /* guests cmpxchg8b have to be emulated atomically */
> + if (bytes == 8) {
>
It's only needed on i386, as 8-byte copy_to_user should be atomic on x86_64.
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
