Re: [PATCH for-4.5 7/8] xen/irq: Handle multiple action per IRQ

From: "Jan Beulich" <JBeulich@suse.com>
To: Julien Grall <julien.grall@linaro.org>
Cc: Keir Fraser <keir@xen.org>,
	Ian Campbell <Ian.Campbell@citrix.com>,
	patches@linaro.org, tim@xen.org, stefano.stabellini@citrix.com,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH for-4.5 7/8] xen/irq: Handle multiple action per IRQ
Date: Tue, 11 Mar 2014 16:08:45 +0000	[thread overview]
Message-ID: <531F431D0200007800122EC3@nat28.tlf.novell.com> (raw)
In-Reply-To: <531F28D5.1000901@linaro.org>

>>> On 11.03.14 at 16:16, Julien Grall <julien.grall@linaro.org> wrote:
> Hello Jan,
> 
> On 02/24/2014 02:48 PM, Julien Grall wrote:
>> On 02/24/2014 02:32 PM, Jan Beulich wrote:
>>>>>> On 24.02.14 at 15:08, Julien Grall <julien.grall@citrix.com> wrote:
>>>> (Adding Jan for x86 part).
>>>>
>>>> On 02/20/2014 09:29 PM, Julien Grall wrote:
>>>>> Hi Ian,
>>>>>
>>>>> On 02/19/2014 11:55 AM, Ian Campbell wrote:
>>>>>> On Fri, 2014-01-24 at 16:43 +0000, Julien Grall wrote:
>>>>>>> On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same
>>>>>>> interrupt.
>>>>>>
>>>>>> Mention here that you are therefore creating a linked list of actions
>>>>>> for each interrupt.
>>>>>>
>>>>>> If you use xen/list.h for this then you get a load of helpers and
>>>>>> iterators which would save you open coding them.
>>>>>
>>>>> After thinking, using xen/list.h won't really remove open code, except
>>>>> removing "action_ptr" in release_dt_irq.
>>>>>
>>>>> Calling release_dt_irq to an IRQ with multiple action shouldn't be
>>>>> called often. Therefore, having both prev and next is a waste of space.
>>>>
>>>> Jan, as it's common code, do you have any thoughts?
>>>
>>> In fact I'm not convinced this action chaining is correct in the first
>>> place, as mentioned by Ian too (considering the potential sharing
>>> between hypervisor and guest). Furthermore, if this is really just
>>> about IOMMU handlers, why can't the SMMU code register a single
>>> action and disambiguate by the dev_id argument passed to the
>>> handler?
>> 
>> The patch #3 of this serie protects the IRQ to be shared with the domain.
>> 
>> I should have remove "eg ARM SMMU" in the description. ARM SMMU is not
>> the only the case, we don't know in advance if the IRQ will be shared
>> (except browsing the DT and checking if this IRQ was used by another
>> devices...). We may have the same thing with other devices.
>> 
>> The logic is painful to handle internally in ARM SMMU driver while we
>> can handle it generically. No need to duplicate the code when a new
>> driver will have the same problem.
> 
> I haven't heard any answer from you. Shall I take as a "go"?

I'm sorry, this got lost between other stuff. Honestly I'm still not
convinced generic multi-action IRQ support is indeed useful. But
I wouldn't veto it either if others are convinced of this approach.

An option possibly to explore might be to have a per-arch trigger
enabling this, and keep it off for x86.

Jan