kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Avi Kivity <avi-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Alexander Graf <agraf-l3A5Bk7waGM@public.gmane.org>
Cc: Marcelo Tosatti
	<mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"carsteno-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org"
	<carsteno-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH 0/7] Consolidate vcpu ioctl locking
Date: Sat, 15 May 2010 20:30:31 +0300	[thread overview]
Message-ID: <4BEEDA37.2080209@redhat.com> (raw)
In-Reply-To: <20442124-2400-4273-A256-6846017D3141-l3A5Bk7waGM@public.gmane.org>

On 05/15/2010 11:26 AM, Alexander Graf wrote:
>
>> That means you never inject an interrupt from the iothread (or from a different vcpu thread)?
>>
>> If that's the case we might make it part of the API and require the ioctl to be issued from the vcpu thread.  We'd still be left with the s390 exception.
>>      
> Well I'm not sure that's guaranteed for MOL or Dolphin, but I guess the user base is small enough to ignore them.
>
> Either way, I'm actually rather unhappy with the way interrupts work right now. We're only injecting interrupts when in the main loop, which is rare if we did our homework right. So what I'd really like to see is that the MPIC on ppc directly invokes KVM_INTERRUPT to pull (and losen) the interrupt line. That way we can't just accidently miss interrupts.
>    

on x86 we signal the vcpu thread to pull it out of the main loop and 
poll the apic.

> What happens now with ppc64 guests that have EE lazily disabled is:
>
> * device in userspace wants to trigger an interrupt
> * mpic pulls external interrupt line
>    

<signal>

> * kvm_arch_pre_run sees that external interrupt line is pulled, issues ioctl(KVM_INTERRUPT), kernel space sets trigger_interrupt=1
> * we enter the guest
> * we inject an external interrupt, set trigger_interrupt=0
> * guest gets the interrupt, sees it's lazily disabled, sets msr.ee=0
> * guest moves on, sets msr.ee=1 again later
> *** guest expects the interrupt to trigger again, but we don't know that it's still pending
>    

Why does the guest expect the interrupt to trigger again?  Is it level 
triggered until acknowledged?

That's exactly why x86 has run->request_interrupt_window, 
run->ready_for_interrupt_injection, and run->if_flag.

> ->  we need to exit to userspace to realize that the interrupt is still active
>
> This is fundamentally broken. What I'd like to see is:
>
> * device in userspace wants to trigger an interrupt
> * mpic pulls external interrupt line, automatically issues ioctl(KVM_INTERRUPT)
> * we enter the guest
> * we inject the external interrupt
> * guest gets the interrupt, sees it's lazily disabled, sets msr.ee=0
> * guest moves on, sets msr.ee=1 again later
> * we inject the external interrupt again, since it's still active
> * guest acknowledges the interrupt with the mpic, issues ioctl(KVM_INTERRUPT, disable)
> ->  all is great
>    

This is similar to KVM_IRQ_LINE.

> For that to work we need to enable triggering KVM_INTERRUPT from a non-vcpu thread.
>    

KVM_IRQ_LINE is a vm ioctl instead of a vcpu ioctl.

The motivation for the strict 'issue vcpu ioctls from vcpu thread' rule 
is to prepare the way for a syscall (instead of ioctl) API.  Then we 
lost the fd argument, and choosing the vcpu is done by associating it 
with the current task.  That allows us to get rid of vcpu->mutex and 
fget_light() (as well as the ioctl dispatch).

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

  parent reply	other threads:[~2010-05-15 17:30 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-13 11:17 [PATCH 0/7] Consolidate vcpu ioctl locking Avi Kivity
2010-05-13 11:17 ` [PATCH 1/7] KVM: PPC: Add missing vcpu_load()/vcpu_put() in vcpu ioctls Avi Kivity
     [not found] ` <1273749459-622-1-git-send-email-avi-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-13 11:17   ` [PATCH 2/7] KVM: x86: Add missing locking to arch specific " Avi Kivity
2010-05-13 11:17   ` [PATCH 4/7] KVM: x86: Lock arch specific vcpu ioctls centrally Avi Kivity
2010-05-13 11:17   ` [PATCH 5/7] KVM: s390: Centrally lock arch specific vcpu ioctls Avi Kivity
2010-05-13 11:17   ` [PATCH 6/7] KVM: PPC: Centralize locking of " Avi Kivity
2010-05-13 11:57   ` [PATCH 0/7] Consolidate vcpu ioctl locking Alexander Graf
2010-05-13 12:01     ` Avi Kivity
2010-05-13 12:03       ` Avi Kivity
     [not found]         ` <4BEBEA7E.80202-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-13 12:03           ` Avi Kivity
     [not found]             ` <4BEBEAAE.9030502-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-13 12:18               ` Alexander Graf
     [not found]                 ` <24423079-CDE0-4DEA-BC73-3B6976BE0CA6-l3A5Bk7waGM@public.gmane.org>
2010-05-13 12:29                   ` Avi Kivity
2010-05-13 19:49                     ` Alexander Graf
     [not found]                       ` <B2627FBE-BB5E-45E2-8E67-E5859B6380A5-l3A5Bk7waGM@public.gmane.org>
2010-05-15  6:16                         ` Avi Kivity
     [not found]                           ` <4BEE3C56.2070007-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-15  6:21                             ` Alexander Graf
     [not found]                               ` <F7406BC6-90A8-43B9-A57F-6B9350B6D356-l3A5Bk7waGM@public.gmane.org>
2010-05-15  7:59                                 ` Avi Kivity
     [not found]                                   ` <4BEE544B.50405-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-15  8:26                                     ` Alexander Graf
     [not found]                                       ` <20442124-2400-4273-A256-6846017D3141-l3A5Bk7waGM@public.gmane.org>
2010-05-15 17:30                                         ` Avi Kivity [this message]
     [not found]                                           ` <4BEEDA37.2080209-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-16  1:00                                             ` Alexander Graf
     [not found]                                               ` <6BE91F3A-C60C-47C0-9EA4-E5F5971B09C2-l3A5Bk7waGM@public.gmane.org>
2010-05-16  8:23                                                 ` Avi Kivity
     [not found]                                                   ` <4BEFAB6D.9000904-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-16  9:01                                                     ` Alexander Graf
     [not found]                                                       ` <DE2111D3-1AC9-45A3-A2BE-B6D012ECCAFE-l3A5Bk7waGM@public.gmane.org>
2010-05-16  9:09                                                         ` Avi Kivity
     [not found]                                                           ` <4BEFB666.50107-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-16  9:35                                                             ` Alexander Graf
     [not found]                                                               ` <04ED5A08-BE13-4C60-B152-EA5541975779-l3A5Bk7waGM@public.gmane.org>
2010-05-16  9:47                                                                 ` Avi Kivity
     [not found]                                                                   ` <4BEFBF42.6020208-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-05-16 10:19                                                                     ` Alexander Graf
2010-05-21  7:35                                         ` Carsten Otte
2010-05-13 11:17 ` [PATCH 3/7] KVM: move vcpu locking to dispatcher for generic vcpu ioctls Avi Kivity
2010-05-15  0:03   ` Marcelo Tosatti
     [not found]     ` <20100515000316.GD2502-I4X2Mt4zSy4@public.gmane.org>
2010-05-16 11:22       ` Avi Kivity
2010-05-13 11:17 ` [PATCH 7/7] KVM: Consolidate arch specific vcpu ioctl locking Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BEEDA37.2080209@redhat.com \
    --to=avi-h+wxahxf7alqt0dzr+alfa@public.gmane.org \
    --cc=agraf-l3A5Bk7waGM@public.gmane.org \
    --cc=carsteno-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org \
    --cc=kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mtosatti-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).