From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:59978)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WFTEN-0004Rq-Td
	for qemu-devel@nongnu.org; Mon, 17 Feb 2014 13:47:58 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WFTEH-0004gJ-Tz
	for qemu-devel@nongnu.org; Mon, 17 Feb 2014 13:47:51 -0500
Received: from mx1.redhat.com ([209.132.183.28]:58588)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WFTEH-0004g9-KP
	for qemu-devel@nongnu.org; Mon, 17 Feb 2014 13:47:45 -0500
Message-ID: <53024F93.3040709@redhat.com>
Date: Mon, 17 Feb 2014 19:06:11 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <20140211182330.GC29329@ERROL.INI.CMU.EDU>
	<20140211195444.GB10951@redhat.com>
	<20140214211311.GH29329@ERROL.INI.CMU.EDU>
	<E2AE3231-DDA6-4EE0-A466-8B0DE4F4DDA7@suse.de>
	<20140214220600.GI29329@ERROL.INI.CMU.EDU>
	<2CEB9F8C-E983-4182-A514-44EC568E18D8@suse.de>
	<20140216114151.GB30056@redhat.com>
	<1392562020.15608.437.camel@ul30vt.home>
	<20140216162300.GI30056@redhat.com>
	<20140217175659.GP29329@ERROL.INI.CMU.EDU>
	<20140217180136.GQ29329@ERROL.INI.CMU.EDU>
In-Reply-To: <20140217180136.GQ29329@ERROL.INI.CMU.EDU>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] RFC: ioapic polarity vs. qemu os-x guest
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Gabriel L. Somlo" <gsomlo@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>, "eddie.dong@intel.com" <eddie.dong@intel.com>, Alexander Graf <agraf@suse.de>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>

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
>