Messages in this thread | | | Date | Wed, 17 Oct 2007 17:09:20 +0200 | From | Laurent Vivier <> | 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@bull.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@bull.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |