Re: [PATCH v4 9/9] KVM: Allow host IRQ sharing for passed-through PCI 2.3 devices

[Avi Kivity <avi@redhat.com>]

On 11/09/2010 04:11 PM, Jan Kiszka wrote:
> >
> >>>   but what about PCI_COMMAND_INTX_DISABLE?
> >>>
> >>>   Perhaps we can hook the kernel's handler for this bit?
> >>
> >>  Some IRQ registration notifier that would allow us to reregister our
> >>  handler with IRQ sharing support? Maybe.
> >
> >  Adding an internal API if preferable to an external one (it may be a
> >  pain to kvm-kmod users though).
>
> Primary concern should be a clean and robust API. From that POV, I would
> prefer an official hook with genirq maintainer blessing over fragile
> detection heuristics in kvm.

That is what I meant (internal to the kernel, not to kvm, vs the 
external kvm/user interface).

> Given that VSIO should benefit from any
> transparent pattern we develop here as well, it's probably worth to go
> that path - if it is really preferred over manual control like this
> patch proposes.

Yes.  The API is already sufficiently complicated that it is very hard 
for userspace to get right.

> For kvm-kmod, we could simply enforce IRQ sharing measures
> unconditionally. Not optimal from the performance POV, but people
> concerned that much about performance should better use KVM over the
> corresponding kernel anyway.

Or backport the needed patches.  kvm-kmod is now no longer needed for 
enterprise users with fixed distros, it's mostly useful for the embedded 
guys which have more flexibility.

> >>>
> >>>   I saw no reason this can't be a spinlock, but perhaps I missed
> >>>   something.  This would allow us to avoid srcu, which is slightly more
> >>>   expensive than rcu.  Since pci 2.3 assigned devices are not a major use
> >>>   case, I'd like not to penalize the mainstream users for this.
> >>
> >>  The lock has to be held across kvm_set_irq, which is the potentially
> >>  expensive (O(n), n == number of VCPUs) operation.
> >
> >  What we should probably do is have broadcast interrupts deferred to a
> >  thread.  I agree it isn't pretty.
>
> Not sure we could defer it that easily (if at all). However, I think
> such improvements should be done on top if this already complex change.

That means flipping locking to srcu and back to rcu.  What I'm proposing 
is to keep broadcast within the spinlock for now, and move it later, 
provided we agree that it's doable.

-- 
error compiling committee.c: too many arguments to function