On Wed, Nov 28, 2012 at 02:22:45PM +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 28, 2012 at 02:13:01PM +0200, Gleb Natapov wrote:
> > On Wed, Nov 28, 2012 at 01:56:16PM +0200, Michael S. Tsirkin wrote:
> > > On Wed, Nov 28, 2012 at 01:43:34PM +0200, Gleb Natapov wrote:
> > > > On Wed, Oct 17, 2012 at 06:06:06PM +0200, Michael S. Tsirkin wrote:
> > > > > We can deliver certain interrupts, notably MSI,
> > > > > from atomic context. Use kvm_set_irq_inatomic,
> > > > > to implement an irq handler for msi.
> > > > >
> > > > > This reduces the pressure on scheduler in case
> > > > > where host and guest irq share a host cpu.
> > > > >
> > > > > Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> > > > > ---
> > > > > virt/kvm/assigned-dev.c | 36 ++++++++++++++++++++++++++----------
> > > > > 1 file changed, 26 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> > > > > index 23a41a9..3642239 100644
> > > > > --- a/virt/kvm/assigned-dev.c
> > > > > +++ b/virt/kvm/assigned-dev.c
> > > > > @@ -105,6 +105,15 @@ static irqreturn_t kvm_assigned_dev_thread_intx(int irq, void *dev_id)
> > > > > }
> > > > >
> > > > > #ifdef __KVM_HAVE_MSI
> > > > > +static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id)
> > > > > +{
> > > > > + struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
> > > > > + int ret = kvm_set_irq_inatomic(assigned_dev->kvm,
> > > > > + assigned_dev->irq_source_id,
> > > > > + assigned_dev->guest_irq, 1);
> > > > Why not use kvm_set_msi_inatomic() and drop kvm_set_irq_inatomic() from
> > > > previous patch?
> > >
> > > kvm_set_msi_inatomic needs a routing entry, and
> > > we don't have the routing entry at this level.
> > >
> > Yes, right. BTW is this interface will be used only for legacy assigned
> > device or there will be other users too?
>
> I think long term we should convert irqfd to this too.
>
VIFO uses irqfd, no? So, why legacy device assignment needs that code
to achieve parity with VFIO? Also why long term? What are the
complications?
> > > Further, guest irq might not be an MSI: host MSI
> > > can cause guest intx injection I think, we need to
> > > bounce it to thread as we did earlier.
> > Ah, so msi in kvm_assigned_dev_msi() is about host msi?
>
> Yes.
>
> > Can host be intx
> > but guest msi?
>
> No.
>
> > You seems to not handle this case. Also injection of intx
> > via ioapic is the same as injecting MSI. The format and the capability
> > of irq message are essentially the same.
>
> Absolutely. So we will be able to extend this to intx long term.
> The difference is in the fact that unlike msi, intx can
> (and does) have multiple entries per GSI.
> I have not yet figured out how to report and handle failure
> in case one of these can be injected in atomic context,
> another can't. There's likely an easy way but can
> be a follow up patch I think.
I prefer to figure that out before introducing the interface. Hmm, we
can get rid of vcpu loop in pic (should be very easily done by checking
for kvm_apic_accept_pic_intr() during apic configuration and keeping
global extint vcpu) and then sorting irq routing entries so that ioapic
entry is first since only ioapic injection can fail.
>
> >
> > >
> > > > > + return unlikely(ret == -EWOULDBLOCK) ? IRQ_WAKE_THREAD : IRQ_HANDLED;
> > > > > +}
> > > > > +
> > > > > static irqreturn_t kvm_assigned_dev_thread_msi(int irq, void *dev_id)
> > > > > {
> > > > > struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
> > > > > @@ -117,6 +126,23 @@ static irqreturn_t kvm_assigned_dev_thread_msi(int irq, void *dev_id)
> > > > > #endif
> > > > >
> > > > > #ifdef __KVM_HAVE_MSIX
> > > > > +static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id)
> > > > > +{
> > > > > + struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
> > > > > + int index = find_index_from_host_irq(assigned_dev, irq);
> > > > > + u32 vector;
> > > > > + int ret = 0;
> > > > > +
> > > > > + if (index >= 0) {
> > > > > + vector = assigned_dev->guest_msix_entries[index].vector;
> > > > > + ret = kvm_set_irq_inatomic(assigned_dev->kvm,
> > > > > + assigned_dev->irq_source_id,
> > > > > + vector, 1);
> > > > > + }
> > > > > +
> > > > > + return unlikely(ret == -EWOULDBLOCK) ? IRQ_WAKE_THREAD : IRQ_HANDLED;
> > > > > +}
> > > > > +
> > > > > static irqreturn_t kvm_assigned_dev_thread_msix(int irq, void *dev_id)
> > > > > {
> > > > > struct kvm_assigned_dev_kernel *assigned_dev = dev_id;
> > > > > @@ -334,11 +360,6 @@ static int assigned_device_enable_host_intx(struct kvm *kvm,
> > > > > }
> > > > >
> > > > > #ifdef __KVM_HAVE_MSI
> > > > > -static irqreturn_t kvm_assigned_dev_msi(int irq, void *dev_id)
> > > > > -{
> > > > > - return IRQ_WAKE_THREAD;
> > > > > -}
> > > > > -
> > > > > static int assigned_device_enable_host_msi(struct kvm *kvm,
> > > > > struct kvm_assigned_dev_kernel *dev)
> > > > > {
> > > > > @@ -363,11 +384,6 @@ static int assigned_device_enable_host_msi(struct kvm *kvm,
> > > > > #endif
> > > > >
> > > > > #ifdef __KVM_HAVE_MSIX
> > > > > -static irqreturn_t kvm_assigned_dev_msix(int irq, void *dev_id)
> > > > > -{
> > > > > - return IRQ_WAKE_THREAD;
> > > > > -}
> > > > > -
> > > > > static int assigned_device_enable_host_msix(struct kvm *kvm,
> > > > > struct kvm_assigned_dev_kernel *dev)
> > > > > {
> > > > > --
> > > > > MST
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > > > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > >
> > > > --
> > > > Gleb.
> >
> > --
> > Gleb.
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
