On Mon, Feb 17, 2014 at 12:57:00PM -0500, Gabriel L. Somlo wrote:
> On Sun, Feb 16, 2014 at 06:23:11PM +0200, Michael S. Tsirkin wrote:
> > Well there is a bigger issue: any interrupt with
> > multiple sources is broken.
> > 
> > __kvm_irq_line_state does a logical OR of all sources,
> > before XOR with polarity.
> > 
> > This makes no sense if polarity is active low.
> 
> So, do you think something like this would make sense, to address
> active-low polarity in __kvm_irq_line_state ?
> (this would be independent of the subsequent xor in
> kvm_ioapic_set_irq()):
 
Make that rather:

-static inline int __kvm_irq_line_state(unsigned long *irq_state,
+static inline int __kvm_irq_line_state(unsigned long *irq_state, int polarity,
                                        int irq_source_id, int level)
{
-        /* Logical OR for level trig interrupt */
        if (level)
                __set_bit(irq_source_id, irq_state);
        else
                __clear_bit(irq_source_id, irq_state);

-       return !!(*irq_state);
+       if (polarity) {
+               /* Logical AND for level trig interrupt, active-low */
+               return !~(*irq_state);
+       } else {
+               /* Logical OR for level trig interrupt, active-high */
+               return !!(*irq_state);
+       }
}

Thanks, and sorry for the noise :)
--Gabriel

Reply via email to