From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 42E58C282C3 for ; Thu, 24 Jan 2019 10:20:17 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 07039218AD for ; Thu, 24 Jan 2019 10:20:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="S58sJjbv"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=ti.com header.i=@ti.com header.b="olp4hUls" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 07039218AD Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=ti.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=gmURoDkF0chZ0MqReY87L92w8FREu4EaBgd6p1vd5Tw=; b=S58sJjbvqMDGp7 EhvQw3EIhf3rlSqsTMeriGXBF/O5nYg8XxZR56PC/Wu9EYT7WWvXC26t+8tp/wer3lkGgnaO8irjv 3JhyoWbjgvXlbCKOARacHKF9BbSVSO9DGOfWV7w4wzf7eF90/aPUocpi0s4+eQLRa2GDolr52hDv+ nzlZwkPWjibaCT//eXK+TcHgazfmCnmealLw13/iziHkQMb8LZ6arWk9fofXfAlHYCjtvXsKkYJcj US4CYB813XByMytLk43FLKmizzGCG9Ycqoewv70epNBTv+yteDLlVvp9mD8IFcBCEuiB8x6a9FWGA ZLTPomxQd1ILEzvZhS0A==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1gmc7M-0006Wz-5c; Thu, 24 Jan 2019 10:20:16 +0000 Received: from lelv0143.ext.ti.com ([198.47.23.248]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1gmc7H-00069g-Sq for linux-arm-kernel@lists.infradead.org; Thu, 24 Jan 2019 10:20:13 +0000 Received: from lelv0265.itg.ti.com ([10.180.67.224]) by lelv0143.ext.ti.com (8.15.2/8.15.2) with ESMTP id x0OAJnWX035146; Thu, 24 Jan 2019 04:19:49 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1548325189; bh=lLgW06B8HXWFIXPwD6DmsaBpUeRnkjI0FRVggopSNQA=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=olp4hUlsboDomK7x4S5nfkIjJHi8kVc/pf1JOe9P56K+XCeM7I+lyWpNf9ArhRish iN/pIw7N4i7B1oQA0n41pk5ymob5ajVngWnNdGFJ0fCrIn+AyykKCkZbka6XUrOn2i v93PQ7ImlMCTwFoDEODzrezYTyfw+eP9WpIVK/3M= Received: from DLEE102.ent.ti.com (dlee102.ent.ti.com [157.170.170.32]) by lelv0265.itg.ti.com (8.15.2/8.15.2) with ESMTPS id x0OAJnpo023610 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 24 Jan 2019 04:19:49 -0600 Received: from DLEE100.ent.ti.com (157.170.170.30) by DLEE102.ent.ti.com (157.170.170.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1591.10; Thu, 24 Jan 2019 04:19:49 -0600 Received: from dlep33.itg.ti.com (157.170.170.75) by DLEE100.ent.ti.com (157.170.170.30) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_RSA_WITH_AES_256_CBC_SHA) id 15.1.1591.10 via Frontend Transport; Thu, 24 Jan 2019 04:19:49 -0600 Received: from [172.24.190.117] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep33.itg.ti.com (8.14.3/8.13.8) with ESMTP id x0OAJjFW001098; Thu, 24 Jan 2019 04:19:46 -0600 Subject: Re: [RFC PATCH v4 08/13] genirq/msi: Add support for allocating single MSI for a device To: Marc Zyngier , Nishanth Menon , Santosh Shilimkar , Rob Herring , "tglx@linutronix.de" , "jason@lakedaemon.net" References: <20181227060829.5080-1-lokeshvutla@ti.com> <20181227061313.5451-1-lokeshvutla@ti.com> <20181227061313.5451-8-lokeshvutla@ti.com> <5be38277-3348-a6d9-4b67-3ead308c009a@arm.com> From: Lokesh Vutla Message-ID: Date: Thu, 24 Jan 2019 15:49:31 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <5be38277-3348-a6d9-4b67-3ead308c009a@arm.com> Content-Language: en-US X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190124_022012_058156_EF842830 X-CRM114-Status: GOOD ( 29.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Device Tree Mailing List , Peter Ujfalusi , Sekhar Nori , "linux-kernel@vger.kernel.org" , Tero Kristo , Linux ARM Mailing List Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Marc, Sorry for the delayed response. Just back from vacation. On 17/01/19 12:00 AM, Marc Zyngier wrote: > On 27/12/2018 06:13, Lokesh Vutla wrote: >> Previously all msi for a device are allocated in one go >> by calling msi_domain_alloc_irq() from a bus layer. This might >> not be the case when a device is trying to allocate interrupts >> dynamically based on a request to it. >> >> So introduce msi_domain_alloc/free_irq() apis to allocate a single >> msi. prepare and activate operations to be handled by bus layer >> calling msi_domain_alloc/free_irq() apis. >> >> Signed-off-by: Lokesh Vutla >> --- >> include/linux/msi.h | 3 +++ >> kernel/irq/msi.c | 62 +++++++++++++++++++++++++++++---------------- >> 2 files changed, 43 insertions(+), 22 deletions(-) >> >> diff --git a/include/linux/msi.h b/include/linux/msi.h >> index 784fb52b9900..474490826f8c 100644 >> --- a/include/linux/msi.h >> +++ b/include/linux/msi.h >> @@ -301,8 +301,11 @@ int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask, >> struct irq_domain *msi_create_irq_domain(struct fwnode_handle *fwnode, >> struct msi_domain_info *info, >> struct irq_domain *parent); >> +int msi_domain_alloc_irq(struct irq_domain *domain, struct device *dev, >> + struct msi_desc *desc, msi_alloc_info_t *arg); >> int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, >> int nvec); >> +void msi_domain_free_irq(struct msi_desc *desc); >> void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev); >> struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain); >> >> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c >> index ad26fbcfbfc8..eb7459324113 100644 >> --- a/kernel/irq/msi.c >> +++ b/kernel/irq/msi.c >> @@ -387,6 +387,35 @@ static bool msi_check_reservation_mode(struct irq_domain *domain, >> return desc->msi_attrib.is_msix || desc->msi_attrib.maskbit; >> } >> >> +int msi_domain_alloc_irq(struct irq_domain *domain, struct device *dev, >> + struct msi_desc *desc, msi_alloc_info_t *arg) >> +{ >> +struct msi_domain_info *info = domain->host_data; >> +struct msi_domain_ops *ops = info->ops; >> +int i, ret, virq; >> + >> +ops->set_desc(arg, desc); >> + >> +virq = __irq_domain_alloc_irqs(domain, -1, desc->nvec_used, >> + dev_to_node(dev), arg, false, >> + desc->affinity); >> +if (virq < 0) { >> +ret = -ENOSPC; >> +if (ops->handle_error) >> +ret = ops->handle_error(domain, desc, ret); >> +if (ops->msi_finish) >> +ops->msi_finish(arg, ret); >> +return ret; >> +} >> + >> +for (i = 0; i < desc->nvec_used; i++) { >> +irq_set_msi_desc_off(virq, i, desc); >> +irq_debugfs_copy_devname(virq + i, dev); >> +} >> + >> +return 0; >> +} >> + >> /** >> * msi_domain_alloc_irqs - Allocate interrupts from a MSI interrupt domain >> * @domain:The domain to allocate from >> @@ -404,7 +433,7 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, >> struct irq_data *irq_data; >> struct msi_desc *desc; >> msi_alloc_info_t arg; >> -int i, ret, virq; >> +int ret, virq; >> bool can_reserve; >> >> ret = msi_domain_prepare_irqs(domain, dev, nvec, &arg); >> @@ -412,24 +441,9 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, >> return ret; >> >> for_each_msi_entry(desc, dev) { >> -ops->set_desc(&arg, desc); >> - >> -virq = __irq_domain_alloc_irqs(domain, -1, desc->nvec_used, >> - dev_to_node(dev), &arg, false, >> - desc->affinity); >> -if (virq < 0) { >> -ret = -ENOSPC; >> -if (ops->handle_error) >> -ret = ops->handle_error(domain, desc, ret); >> -if (ops->msi_finish) >> -ops->msi_finish(&arg, ret); >> +ret = msi_domain_alloc_irq(domain, dev, desc, &arg); >> +if (ret) >> return ret; >> -} >> - >> -for (i = 0; i < desc->nvec_used; i++) { >> -irq_set_msi_desc_off(virq, i, desc); >> -irq_debugfs_copy_devname(virq + i, dev); >> -} >> } >> >> if (ops->msi_finish) >> @@ -487,6 +501,12 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, >> return ret; >> } >> >> +void msi_domain_free_irq(struct msi_desc *desc) >> +{ >> +irq_domain_free_irqs(desc->irq, desc->nvec_used); >> +desc->irq = 0; >> +} >> + >> /** >> * msi_domain_free_irqs - Free interrupts from a MSI interrupt @domain associated tp @dev >> * @domain:The domain to managing the interrupts >> @@ -503,10 +523,8 @@ void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev) >> * enough that there is no IRQ associated to this >> * entry. If that's the case, don't do anything. >> */ >> -if (desc->irq) { >> -irq_domain_free_irqs(desc->irq, desc->nvec_used); >> -desc->irq = 0; >> -} >> +if (desc->irq) >> +msi_domain_free_irq(desc); >> } >> } >> >> > > I can see some interesting issues with this API. > > At the moment, MSIs are allocated upfront, and that's usually done > before the driver can do anything else. With what you're suggesting > here, MSIs can now be allocated at any time, which sounds great. But how > does it work when MSIs get added/freed in parallel? I can't see any > locking here... > > It is also pretty nasty that the user of this API has to know about the > MSI descriptor. Really, nobody should have to deal with this outside of > the MSI layer. > > The real question is why you need to need to allocate MSIs on demand for > a given device. Usually, you allocate them because this is a per-CPU > resource, or something similar. What makes it so variable that you need > to resort to fine grained MSI allocation? I added this after the discussion we had in the previous version[1] of this series. Let me provide the details again: As you must be aware INTR is interrupt re-director and INTA is the interrupt multiplexer is the SoC. Here we are trying to address the interrupt connection route as below: Device(Global event) --> INTA --> INTR --> GIC For the above case you suggested to have the following sw IRQ domain hierarchy: INTA multi MSI --> INTA --> INTR --> GIC The problem here with the INTA MSI is that all the interrupts for a device should be pre-allocated during the device probe time. But this is not what we wanted especially for the DMA case. An example DMA ring connection would look like below[2]: +---------------------+ | IA | +--------+ | +------+ | +--------+ +------+ | ring 1 +----->evtA+->VintX+-------->+ IR +-- ---> GIC +--> +--------+ | +------+ | +--------+ +------+ Linux IRQ Y evtA | | | | +----------------------+ So when a DMA client driver requests a dma channel during probe, the DMA driver gets a free ring in its allocated range. Then DMA driver requests MSI layer for an IRQ. This is why I had to introduce on demand allocation of MSIs for a device. The reason why we avoided DMA driver to allocate interrupts during its probe as it is not aware of the exact no of channels that are going to be used. Also max allocation of interrupts will overrun the gic IRQs available to this INTA and the IPs that are connected to INTR directly will not get any interrupts. I hope this is clear. [1] https://lkml.org/lkml/2018/11/5/894 [2] https://pastebin.ubuntu.com/p/RTrgzfCMby/ Thanks and regards, Lokesh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel