From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH for-4.5 7/8] xen/irq: Handle multiple action per IRQ Date: Mon, 17 Mar 2014 21:05:13 +0000 Message-ID: <53276389.8080109@linaro.org> References: <1390581822-32624-1-git-send-email-julien.grall@linaro.org> <1390581822-32624-8-git-send-email-julien.grall@linaro.org> <1392810905.29739.19.camel@kazak.uk.xensource.com> <530673BD.9010301@linaro.org> <530B5275.7010008@citrix.com> <530B6606020000780011ED39@nat28.tlf.novell.com> <530B5BAC.2010100@linaro.org> <531F28D5.1000901@linaro.org> <531F431D0200007800122EC3@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WPeik-0000lF-21 for xen-devel@lists.xenproject.org; Mon, 17 Mar 2014 21:05:18 +0000 Received: by mail-wi0-f182.google.com with SMTP id d1so2828984wiv.9 for ; Mon, 17 Mar 2014 14:05:15 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini , Jan Beulich Cc: Keir Fraser , Ian Campbell , patches@linaro.org, tim@xen.org, stefano.stabellini@citrix.com, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org Hi Stefano, On 17/03/14 19:06, Stefano Stabellini wrote: > On Tue, 11 Mar 2014, Jan Beulich wrote: >>>>> On 11.03.14 at 16:16, Julien Grall 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 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. > > I agree. > In general if an IRQ is shared among multiple devices, it is likely to > go to Dom0 and have a single action from Xen point of view. > An IRQ shared between Xen and a guest is a very bad idea. I guess you agree with Jan, rigth? If so, I think you misunderstood the goal of this patch. This patch does *NOT* add support for IRQ sharing between domains and Xen (patch #3 is preventing that). Some devices are describing a same interrupt twice in the device tree and the action is not the same to accomplish. For instance for the SMMU on midway, the device tree bindings is: smmu_sata: smmu@9,20180000 { compatible = "arm,mmu-400"; reg = <0x9 0x20180000 0x10000>; mmu-masters = <&sata 0 1 2 3 4 5 6 7 8 9>; #global-interrupts = <1>; interrupts = <0 114 4 0 114 4>; calxeda,smmu-secure-config-access; arm,smmu-isolate-devices; }; As you can see the same interrupts is used twice: the first one is used for the global interrupt and the second one as context interrupt. For the latter, each time a new context bank is created (e.g a device is passthrough to IOMMU), we need to register another handler. Now we have 2 solutions to implement it: 1) Implement it directly in the SMMU drivers => We are assuming we wouldn't this situation on another IOMMU drivers 2) Implement it generically The former one is complicated to implement, because it's not fixed if the IRQ will be described twice or not. We can take advantage of the generic code for this purpose. Regards, -- Julien Grall