Gleb Natapov wrote: > On Mon, Jul 05, 2010 at 08:49:31AM +0200, Jan Kiszka wrote: >> Gleb Natapov wrote: >>> On Mon, Jul 05, 2010 at 08:39:38AM +0200, Jan Kiszka wrote: >>>> Paul Brook wrote: >>>>>> Blue Swirl wrote: >>>>>>> On Sat, Jul 3, 2010 at 7:39 AM, Jan Kiszka wrote: >>>>>>>> Paul Brook wrote: >>>>>>>>>> I really see no tangible objection to Jan's patches. They don't >>>>>>>>>> impact any other code. They don't inhibit flexibility in the >>>>>>>>>> infrastructure. You might consider it to be a "hack" but so what. >>>>>>>>>> QEMU is filled with hacks. It would be useless without them because >>>>>>>>>> there would be very little code. >>>>>>>>> I object strongly to anything that makes qemu_irq a message passing >>>>>>>>> API. if you want message passing then you should not be using >>>>>>>>> qemu_irq. >>>>>>>> Blueswirl objected to the straightforward return-value approach I first >>>>>>>> posted. You seems to be more open towards this, right? Still looks like >>>>>>>> I cannot make you both happy at the same time. So what to do? >>>>>>> I have withdrawn my objection. We can do message passing with some >>>>>>> different API later, for simple coalescing needs the return value >>>>>>> approach is enough. >>>>>> Great! I'll respin my patches ASAP. >>>>> Note that I still have some concerns over the semantics of that API. >>>>> I believe this should be fundamentally state based, not event based. >>>> For the caller of qemu_set_irq, it will be like that. >>>> >>> Unfortunately just having qemu_set_irq() return value is not enough to >>> fix timedrift problem for all Windows. For some of them you need to know >>> _which_ CPU accepted IRQ. >> Return values: >> < 0 - no state change, specifically due to masking or latching >> >= 0 - first CPU (lowest index) on which a state change was achieved >> >> Sufficient? >> > To problem specific for such generic interface. Doesn't allow to > distinguish between masking and coalescing. It does (return values < 0 are specified to differentiate between them and more). > Assumes that CPU with > lowest index is BSP (that one we can actually guaranty if we want > to). Well, the generic solution would be returning a bitmap of the CPUs that were affected, but this is impractical. However, at least x86 should be fine with the information "state change also on BSP", e.g. like this: 0 - state change on one or more CPUs, none of them is the BSP 1 - state change on BSP (and possible more CPUs) > And what about PIC mode where interrupt receiver is not CPU? In the end, there's always a CPU (or several of them). Jan