On 2012-05-30 19:41, Michael S. Tsirkin wrote: > On Wed, May 30, 2012 at 07:29:52PM +0200, Jan Kiszka wrote: >> On 2012-05-30 19:20, Michael S. Tsirkin wrote: >>> On Wed, May 30, 2012 at 06:46:03PM +0200, Jan Kiszka wrote: >>>> On 2012-05-30 18:17, Michael S. Tsirkin wrote: >>>>> On Wed, May 30, 2012 at 05:47:45PM +0200, Jan Kiszka wrote: >>>>>> On 2012-05-30 16:59, Michael S. Tsirkin wrote: >>>>>>> On Wed, May 30, 2012 at 04:46:14PM +0200, Jan Kiszka wrote: >>>>>>>> On 2012-05-30 16:42, Michael S. Tsirkin wrote: >>>>>>>>> On Wed, May 30, 2012 at 04:38:25PM +0200, Jan Kiszka wrote: >>>>>>>>>> On 2012-05-30 15:34, Michael S. Tsirkin wrote: >>>>>>>>>>> Below's as far as I got - hopefully this >>>>>>>>>>> explains what I had in mind: for deep hierarchies >>>>>>>>>>> this will save a bus scan so I think this makes sense >>>>>>>>>>> even in the absense of kvm irqchip. >>>>>>>>>>> >>>>>>>>>>> Warning: completely untested and known to be incomplete. >>>>>>>>>>> Please judge whether this is going in a direction that >>>>>>>>>>> is helpful for your efforts. If you respond in the positive, >>>>>>>>>>> I hope to be able to get back to this next week. >>>>>>>>>> >>>>>>>>>> We need to >>>>>>>>>> - account for polarity changes along the route >>>>>>>>>> - get rid of irq_count as it is no longer updated in the fast path, >>>>>>>>>> thus useless on routing updates >>>>>>>>> >>>>>>>>> I'll need to consider this more to understand what you mean here. >>>>>>>> >>>>>>>> If, e.g., the host bridge is configured to flip the polarity of some >>>>>>>> interrupt on delivery, the fast path must be able to explore this in >>>>>>>> order to do the same. >>>>>>> >>>>>>> This can be solved incrementally I think - handle polarity >>>>>>> change like mapping change, no? >>>>>> >>>>>> Both belong together if we want to do it generically, IMHO. >>>>> >>>>> So irq_polarity callback like we have for map_irq ATM? But at least pci >>>>> bridges never do this flip. Maybe this is the property of the interrupt >>>>> controller after all? >>>> >>>> It is a property of some host bridges apparently (e.g. bonito). >>> >>> So I'm not sure it's worth it to abstract that but I don't mind either. >> >> It is must for a generic solution. We cannot modeling after a single >> host bridge called PIIX3. >> >>> >>>> In >>>> theory, someone could also add a PCI-PCI bridge that does this (or does >>>> the spec disallow it?). >>> >>> It seems to disallow it. >>> >>>>> >>>>>>> >>>>>>>> Then you may want to have a look at how irq_count is maintained and >>>>>>>> when >>>>>>>> it is used. >>>>>>> >>>>>>> In my patch it simply there to OR irq levels of all devices >>>>>>> connected to a specific pin. >>>>>> >>>>>> I think what we want is to avoid any walks and intermediate state >>>>>> updates for all IRQ deliveries. >>>>> >>>>> Well the bus is not an intermediate state ATM as piix >>>>> only has a bit per IRQ so it can't OR devices together. >>>>> So you want to move the counter over to piix? >>>> >>>> irq_count can't be moved logically as it is part of the vmstate. But it >>>> should only be generated for saving the state by polling all devices >>>> (and bridges). >>>> >>>> For that we need is an optional callback for devices via which we can >>>> ask them to update PCIDevice::irq_state in case they don't trigger >>>> pci_set_irq regularly. >>> >>> Let's worry about migration compatibility separately. >>> >>>> And pci_set_irq could simply change the input >>>> line of the IRQ controller according to the cached route and polarity >>>> mapping. >>> >>> But the line is shared between multiple devices. >>> You need to perform a logical OR function between them all, >>> this is what the counter does. >> >> Needs to be solved at a different level (at the final output of the fast >> path, surely not per PCI bus). >> >> Jan > > Well the final output is in kvm :)
One final output. Userspace originated IRQs can be merged in userspace before flipping KVM's [IOA]PIC input line. > What happens with assigned devices is > that kvm performs the logical OR using source id. But the number of > available source IDs is limited (up to BIT_PER_LONG currently). > > So it's not reasonable to allocate one per emulated device. > > So assigned devices will have to be special-cased somehow. > Given that, is there really a point in moving these bits > around? > > Well, I'll stop ranting here. The patch that I sent is not intrusive > and simply gives you a simple way to implement pci_device_get_host_irq, > also optimizing emulated devices somewhat. So if you think you need > pci_device_get_host_irq I think this is a reasonable way to support > that. But if you changed your mind, I don't mind. Sorry, your patch doesn't help me in any way. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux