Il 17/02/2014 19:01, Gabriel L. Somlo ha scritto:
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);
This is ~*irq_state == 0, i.e. *irq_state == ~0.What if high-order bits of *irq_state are never used? That is, do you need to consider the maximum valid irq_source_id too?
+ } else {
+ /* Logical OR for level trig interrupt, active-high */
+ return !!(*irq_state);
Better rewrite this as *irq_state != 0. Paolo
+ } } Thanks, and sorry for the noise :) --Gabriel
-- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
