[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <471625A0.4020201@bull.net>
Date: Wed, 17 Oct 2007 17:09:20 +0200
From: Laurent Vivier <Laurent.Vivier@...l.net>
To: Avi Kivity <avi@...ranet.com>
Cc: mingo@...e.hu, linux-kernel@...r.kernel.org, borntraeger@...ibm.com
Subject: Re: [PATCH] clear PF_VCPU in kvm_guest_exit()
Avi Kivity wrote:
> Laurent Vivier wrote:
>> clear PF_VCPU in kvm_guest_exit() and move kvm_guest_exit() after
>> local_irq_enable().
>>
>> According comments from Avi, we can clear PF_VCPU in kvm_guest_exit if we move
>> it after local_irq_enable().
>>
>> http://lkml.org/lkml/2007/10/15/114
>>
>> To simplify s390 port, we don't clear it in account_system_time().
>>
>> http://lkml.org/lkml/2007/10/15/183
>>
>> Signed-off-by: Laurent Vivier <Laurent.Vivier@...l.net>
>> ---
>> drivers/kvm/kvm.h | 1 +
>> drivers/kvm/kvm_main.c | 3 ++-
>> kernel/sched.c | 1 -
>> 3 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
>> index e9dbf67..e8b4902 100644
>> --- a/drivers/kvm/kvm.h
>> +++ b/drivers/kvm/kvm.h
>> @@ -677,6 +677,7 @@ static inline void kvm_guest_enter(void)
>>
>> static inline void kvm_guest_exit(void)
>> {
>> + current->flags &= ~PF_VCPU;
>> }
>>
>
>
> Ingo already added this.
>
>>
>> static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva,
>> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
>> index 87275be..a9db477 100644
>> --- a/drivers/kvm/kvm_main.c
>> +++ b/drivers/kvm/kvm_main.c
>> @@ -2194,12 +2194,13 @@ again:
>>
>> kvm_x86_ops->run(vcpu, kvm_run);
>>
>> - kvm_guest_exit();
>> vcpu->guest_mode = 0;
>> local_irq_enable();
>>
>> ++vcpu->stat.exits;
>>
>>
>
> Need a barrier() here. Otherwise the compiler may reorder
> "kvm_guest_exit();" and "++vcpu->stats.exits;", making kvm_guest_exit()
> right after local_irq_enable(). If it inlines kvm_guest_exit(), and
> further encodes it in one instruction (both likely) we get:
>
> sti
> andl $something, (something_else)
For the moment it is:
5676: fb sti
5677: 48 8b 85 f8 fe ff ff mov 0xfffffffffffffef8(%rbp),%rax
567e: 8b 80 10 0c 00 00 mov 0xc10(%rax),%eax
5684: 8d 50 01 lea 0x1(%rax),%edx
5687: 48 8b 85 f8 fe ff ff mov 0xfffffffffffffef8(%rbp),%rax
568e: 89 90 10 0c 00 00 mov %edx,0xc10(%rax)
5694: 65 48 8b 04 25 00 00 mov %gs:0x0,%rax
569b: 00 00
569d: 48 89 45 a8 mov %rax,0xffffffffffffffa8(%rbp)
56a1: 48 8b 45 a8 mov 0xffffffffffffffa8(%rbp),%rax
56a5: 48 89 45 b0 mov %rax,0xffffffffffffffb0(%rbp)
56a9: 48 8b 45 b0 mov 0xffffffffffffffb0(%rbp),%rax
56ad: 48 89 c2 mov %rax,%rdx
56b0: 8b 42 14 mov 0x14(%rdx),%eax
56b3: 83 e0 ef and $0xffffffffffffffef,%eax
56b6: 89 42 14 mov %eax,0x14(%rdx)
So I think it is out of the interrupt shadow... but you're right I will resend
the patch adding barrier() (because in futur gcc can behave differently),
removing the part already added by Ingo and copying kvm-devel.
> The andl executes in interrupt shadow (that is, interrupts are still
> disabled) and the timer tick won't see PF_VCPU.
>
>> + kvm_guest_exit();
>> +
>> preempt_enable();
>>
>> /*
>>
>
>
> Please copy kvm-devel on kvm patches.
>
Laurent
--
---------------- Laurent.Vivier@...l.net -----------------
"Given enough eyeballs, all bugs are shallow" E. S. Raymond
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists