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 :) 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. > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux
