[Qemu-devel] plan for device assignment upstream

[Qemu-devel] plan for device assignment upstream

[Michael S. Tsirkin]

I've been thinking hard about Jan's patches for device
assignment. Basically while I thought it makes sense
to make all devices: assignment and not - behave the
same and use same APIs for injecting irqs, Anthony thinks there is huge
value in making irq propagation hierarchical and device assignment
should be special cased.

We seem to be at impasse for now and I think merging
assignment ASAP has higher value than this specific
issue. So I fold - let's do it as Anthony and Jan's
original patches proposed.

Jan, can you please rebase and repost your original patchset (against
master, not against pci) that added query for host irqs callbacks for
device assignment? I'll re-review ignoring the idea of using the cache,
with intent apply after I'll drop cache code from the pci branch in a
couple of days (busy otherwise now).

I still intend to rework this later on, but that can wait.

Thanks,

-- 
MST

Re: [Qemu-devel] plan for device assignment upstream

[Avi Kivity]

On 07/02/2012 12:18 PM, Michael S. Tsirkin wrote:
> I've been thinking hard about Jan's patches for device
> assignment. Basically while I thought it makes sense
> to make all devices: assignment and not - behave the
> same and use same APIs for injecting irqs, Anthony thinks there is huge
> value in making irq propagation hierarchical and device assignment
> should be special cased.
> 
> We seem to be at impasse for now and I think merging
> assignment ASAP has higher value than this specific
> issue. So I fold - let's do it as Anthony and Jan's
> original patches proposed.
> 
> Jan, can you please rebase and repost your original patchset (against
> master, not against pci) that added query for host irqs callbacks for
> device assignment? I'll re-review ignoring the idea of using the cache,
> with intent apply after I'll drop cache code from the pci branch in a
> couple of days (busy otherwise now).
> 
> I still intend to rework this later on, but that can wait.

Agree with both your ideas about the API and the decision to rework it
in tree.

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

Re: [Qemu-devel] plan for device assignment upstream

[Jan Kiszka]

On 2012-07-02 11:18, Michael S. Tsirkin wrote:
> I've been thinking hard about Jan's patches for device
> assignment. Basically while I thought it makes sense
> to make all devices: assignment and not - behave the
> same and use same APIs for injecting irqs, Anthony thinks there is huge
> value in making irq propagation hierarchical and device assignment
> should be special cased.

On the long term, we will need direct injection, ie. caching, to allow
making it lock-less. Stepping through all intermediate layers will cause
troubles, at least performance-wise, when having to take and drop a lock
at each stop.

> 
> We seem to be at impasse for now and I think merging
> assignment ASAP has higher value than this specific
> issue. So I fold - let's do it as Anthony and Jan's
> original patches proposed.
> 
> Jan, can you please rebase and repost your original patchset (against
> master, not against pci) that added query for host irqs callbacks for
> device assignment? I'll re-review ignoring the idea of using the cache,
> with intent apply after I'll drop cache code from the pci branch in a
> couple of days (busy otherwise now).

OK, will do ASAP.

> 
> I still intend to rework this later on, but that can wait.
> 
> Thanks,
> 

Thanks as well,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] plan for device assignment upstream

[Avi Kivity]

On 07/02/2012 12:30 PM, Jan Kiszka wrote:
> On 2012-07-02 11:18, Michael S. Tsirkin wrote:
>> I've been thinking hard about Jan's patches for device
>> assignment. Basically while I thought it makes sense
>> to make all devices: assignment and not - behave the
>> same and use same APIs for injecting irqs, Anthony thinks there is huge
>> value in making irq propagation hierarchical and device assignment
>> should be special cased.
> 
> On the long term, we will need direct injection, ie. caching, to allow
> making it lock-less. Stepping through all intermediate layers will cause
> troubles, at least performance-wise, when having to take and drop a lock
> at each stop.

So we precalculate everything beforehand.  Instead of each qemu_irq
triggering a callback, calculating the next hop and firing the next
qemu_irq, configure each qemu_irq array with a function that describes
how to take the next hop.  Whenever the configuration changes,
recalculate all routes.

For device assignment or vhost, we can have a qemu_irq_irqfd() which
converts a qemu_irq to an eventfd.  If the route calculations determine
that it can be serviced via a real irqfd, they also configure it as an
irqfd.  Otherwise qemu configures a poll on this eventfd and calls the
callback when needed.


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

Re: [Qemu-devel] plan for device assignment upstream

[Blue Swirl]

On Mon, Jul 2, 2012 at 9:43 AM, Avi Kivity <avi@redhat.com> wrote:
> On 07/02/2012 12:30 PM, Jan Kiszka wrote:
>> On 2012-07-02 11:18, Michael S. Tsirkin wrote:
>>> I've been thinking hard about Jan's patches for device
>>> assignment. Basically while I thought it makes sense
>>> to make all devices: assignment and not - behave the
>>> same and use same APIs for injecting irqs, Anthony thinks there is huge
>>> value in making irq propagation hierarchical and device assignment
>>> should be special cased.
>>
>> On the long term, we will need direct injection, ie. caching, to allow
>> making it lock-less. Stepping through all intermediate layers will cause
>> troubles, at least performance-wise, when having to take and drop a lock
>> at each stop.
>
> So we precalculate everything beforehand.  Instead of each qemu_irq
> triggering a callback, calculating the next hop and firing the next
> qemu_irq, configure each qemu_irq array with a function that describes
> how to take the next hop.  Whenever the configuration changes,
> recalculate all routes.

Yes, we had this discussion last year when I proposed the IRQ matrix:
http://lists.nongnu.org/archive/html/qemu-devel/2011-09/msg00474.html

One problem with the matrix is that it only works for enable/disable
level, not for more complex situations like boolean logic or
multiplexed outputs.

Perhaps the devices should describe the currently valid logic with
packet filter type mechanism? I think that could scale arbitrarily and
it could be more friendly even as a kernel interface?

>
> For device assignment or vhost, we can have a qemu_irq_irqfd() which
> converts a qemu_irq to an eventfd.  If the route calculations determine
> that it can be serviced via a real irqfd, they also configure it as an
> irqfd.  Otherwise qemu configures a poll on this eventfd and calls the
> callback when needed.
>
>
> --
> error compiling committee.c: too many arguments to function
>
>
>

Re: [Qemu-devel] plan for device assignment upstream

[Avi Kivity]

On 07/03/2012 10:06 PM, Blue Swirl wrote:
> On Mon, Jul 2, 2012 at 9:43 AM, Avi Kivity <avi@redhat.com> wrote:
>> On 07/02/2012 12:30 PM, Jan Kiszka wrote:
>>> On 2012-07-02 11:18, Michael S. Tsirkin wrote:
>>>> I've been thinking hard about Jan's patches for device
>>>> assignment. Basically while I thought it makes sense
>>>> to make all devices: assignment and not - behave the
>>>> same and use same APIs for injecting irqs, Anthony thinks there is huge
>>>> value in making irq propagation hierarchical and device assignment
>>>> should be special cased.
>>>
>>> On the long term, we will need direct injection, ie. caching, to allow
>>> making it lock-less. Stepping through all intermediate layers will cause
>>> troubles, at least performance-wise, when having to take and drop a lock
>>> at each stop.
>>
>> So we precalculate everything beforehand.  Instead of each qemu_irq
>> triggering a callback, calculating the next hop and firing the next
>> qemu_irq, configure each qemu_irq array with a function that describes
>> how to take the next hop.  Whenever the configuration changes,
>> recalculate all routes.
> 
> Yes, we had this discussion last year when I proposed the IRQ matrix:
> http://lists.nongnu.org/archive/html/qemu-devel/2011-09/msg00474.html
> 
> One problem with the matrix is that it only works for enable/disable
> level, not for more complex situations like boolean logic or
> multiplexed outputs.

I think we do need to support inverters etc.

> Perhaps the devices should describe the currently valid logic with
> packet filter type mechanism? I think that could scale arbitrarily and
> it could be more friendly even as a kernel interface?

Interesting idea.  So qemu creates multiple eventfds, gives half to
devices and half to kvm (as irqfds), and configures bpf programs that
calculate the irqfd outputs from the vfio inputs.

At least for x86 this is overkill.  I would be okay with
one-input-one-output cases handled with the current code and everything
else routed through qemu.

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

Re: [Qemu-devel] plan for device assignment upstream

[Blue Swirl]

On Wed, Jul 4, 2012 at 8:05 AM, Avi Kivity <avi@redhat.com> wrote:
> On 07/03/2012 10:06 PM, Blue Swirl wrote:
>> On Mon, Jul 2, 2012 at 9:43 AM, Avi Kivity <avi@redhat.com> wrote:
>>> On 07/02/2012 12:30 PM, Jan Kiszka wrote:
>>>> On 2012-07-02 11:18, Michael S. Tsirkin wrote:
>>>>> I've been thinking hard about Jan's patches for device
>>>>> assignment. Basically while I thought it makes sense
>>>>> to make all devices: assignment and not - behave the
>>>>> same and use same APIs for injecting irqs, Anthony thinks there is huge
>>>>> value in making irq propagation hierarchical and device assignment
>>>>> should be special cased.
>>>>
>>>> On the long term, we will need direct injection, ie. caching, to allow
>>>> making it lock-less. Stepping through all intermediate layers will cause
>>>> troubles, at least performance-wise, when having to take and drop a lock
>>>> at each stop.
>>>
>>> So we precalculate everything beforehand.  Instead of each qemu_irq
>>> triggering a callback, calculating the next hop and firing the next
>>> qemu_irq, configure each qemu_irq array with a function that describes
>>> how to take the next hop.  Whenever the configuration changes,
>>> recalculate all routes.
>>
>> Yes, we had this discussion last year when I proposed the IRQ matrix:
>> http://lists.nongnu.org/archive/html/qemu-devel/2011-09/msg00474.html
>>
>> One problem with the matrix is that it only works for enable/disable
>> level, not for more complex situations like boolean logic or
>> multiplexed outputs.
>
> I think we do need to support inverters etc.
>
>> Perhaps the devices should describe the currently valid logic with
>> packet filter type mechanism? I think that could scale arbitrarily and
>> it could be more friendly even as a kernel interface?
>
> Interesting idea.  So qemu creates multiple eventfds, gives half to
> devices and half to kvm (as irqfds), and configures bpf programs that
> calculate the irqfd outputs from the vfio inputs.

I wasn't thinking of using fds, I guess that could work too but just
that the interface could be similar to packet filters. So a device
which implements an enable switch and ORs 8 inputs to a global output
could be implemented with:
context = rule_init();
context = append_rule(context, R_OR, 8, &irq_array[]);
context = append_rule(context, R_AND, 1, irq_enable);
send_to_kernel_or_master_irq_controller(context);

>
> At least for x86 this is overkill.  I would be okay with
> one-input-one-output cases handled with the current code and everything
> else routed through qemu.

If this is efficient, some of the internal logic inside devices (for
example PCI) could be implemented with the rules. Usually devices have
one or just a few IRQ outputs but several possible internal sources
for these.

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

Re: [Qemu-devel] plan for device assignment upstream

[Michael S. Tsirkin]

On Mon, Jul 02, 2012 at 12:43:48PM +0300, Avi Kivity wrote:
> On 07/02/2012 12:30 PM, Jan Kiszka wrote:
> > On 2012-07-02 11:18, Michael S. Tsirkin wrote:
> >> I've been thinking hard about Jan's patches for device
> >> assignment. Basically while I thought it makes sense
> >> to make all devices: assignment and not - behave the
> >> same and use same APIs for injecting irqs, Anthony thinks there is huge
> >> value in making irq propagation hierarchical and device assignment
> >> should be special cased.
> > 
> > On the long term, we will need direct injection, ie. caching, to allow
> > making it lock-less. Stepping through all intermediate layers will cause
> > troubles, at least performance-wise, when having to take and drop a lock
> > at each stop.
> 
> So we precalculate everything beforehand.  Instead of each qemu_irq
> triggering a callback, calculating the next hop and firing the next
> qemu_irq, configure each qemu_irq array with a function that describes
> how to take the next hop.  Whenever the configuration changes,
> recalculate all routes.
> 
> For device assignment or vhost, we can have a qemu_irq_irqfd() which
> converts a qemu_irq to an eventfd.  If the route calculations determine
> that it can be serviced via a real irqfd, they also configure it as an
> irqfd.  Otherwise qemu configures a poll on this eventfd and calls the
> callback when needed.

This is more or less what I had in mind and what Anthony objects to.

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

Re: [Qemu-devel] plan for device assignment upstream

[Avi Kivity]

On 07/04/2012 01:42 PM, Michael S. Tsirkin wrote:
> On Mon, Jul 02, 2012 at 12:43:48PM +0300, Avi Kivity wrote:
>> On 07/02/2012 12:30 PM, Jan Kiszka wrote:
>> > On 2012-07-02 11:18, Michael S. Tsirkin wrote:
>> >> I've been thinking hard about Jan's patches for device
>> >> assignment. Basically while I thought it makes sense
>> >> to make all devices: assignment and not - behave the
>> >> same and use same APIs for injecting irqs, Anthony thinks there is huge
>> >> value in making irq propagation hierarchical and device assignment
>> >> should be special cased.
>> > 
>> > On the long term, we will need direct injection, ie. caching, to allow
>> > making it lock-less. Stepping through all intermediate layers will cause
>> > troubles, at least performance-wise, when having to take and drop a lock
>> > at each stop.
>> 
>> So we precalculate everything beforehand.  Instead of each qemu_irq
>> triggering a callback, calculating the next hop and firing the next
>> qemu_irq, configure each qemu_irq array with a function that describes
>> how to take the next hop.  Whenever the configuration changes,
>> recalculate all routes.
>> 
>> For device assignment or vhost, we can have a qemu_irq_irqfd() which
>> converts a qemu_irq to an eventfd.  If the route calculations determine
>> that it can be serviced via a real irqfd, they also configure it as an
>> irqfd.  Otherwise qemu configures a poll on this eventfd and calls the
>> callback when needed.
> 
> This is more or less what I had in mind and what Anthony objects to.

Can you post an interface that supports this?  Then we can see exactly
what is objectionable.


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

Re: [Qemu-devel] plan for device assignment upstream

[Michael S. Tsirkin]

On Wed, Jul 04, 2012 at 02:24:45PM +0300, Avi Kivity wrote:
> On 07/04/2012 01:42 PM, Michael S. Tsirkin wrote:
> > On Mon, Jul 02, 2012 at 12:43:48PM +0300, Avi Kivity wrote:
> >> On 07/02/2012 12:30 PM, Jan Kiszka wrote:
> >> > On 2012-07-02 11:18, Michael S. Tsirkin wrote:
> >> >> I've been thinking hard about Jan's patches for device
> >> >> assignment. Basically while I thought it makes sense
> >> >> to make all devices: assignment and not - behave the
> >> >> same and use same APIs for injecting irqs, Anthony thinks there is huge
> >> >> value in making irq propagation hierarchical and device assignment
> >> >> should be special cased.
> >> > 
> >> > On the long term, we will need direct injection, ie. caching, to allow
> >> > making it lock-less. Stepping through all intermediate layers will cause
> >> > troubles, at least performance-wise, when having to take and drop a lock
> >> > at each stop.
> >> 
> >> So we precalculate everything beforehand.  Instead of each qemu_irq
> >> triggering a callback, calculating the next hop and firing the next
> >> qemu_irq, configure each qemu_irq array with a function that describes
> >> how to take the next hop.  Whenever the configuration changes,
> >> recalculate all routes.
> >> 
> >> For device assignment or vhost, we can have a qemu_irq_irqfd() which
> >> converts a qemu_irq to an eventfd.  If the route calculations determine
> >> that it can be serviced via a real irqfd, they also configure it as an
> >> irqfd.  Otherwise qemu configures a poll on this eventfd and calls the
> >> callback when needed.
> > 
> > This is more or less what I had in mind and what Anthony objects to.
> 
> Can you post an interface that supports this?  Then we can see exactly
> what is objectionable.


There wasn't a patch, just discussion. I'll try to sketch
an API so people can respond more specifically.

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