Re: RFC: ioapic polarity vs. qemu os-x guest

[Paolo Bonzini <pbonzini@redhat.com>]

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
>