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=-14.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS 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 EEFAEC47082 for ; Sat, 5 Jun 2021 20:14:54 +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 B46AD613B4 for ; Sat, 5 Jun 2021 20:14:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B46AD613B4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References: List-Owner; bh=IrNipz0ocSPZ2aBtNEAOhqjv7dsKhSlF+PjXTDJwmdI=; b=wFBOsnkftDR0ye odojA6q/pWi8m0kPH1AzIT1jFc6qvchwn4CM3/49RnrQ9Rms8dtcWE/THGz303jIx0QdWUK5fJQta Fsotcdc9xF6yda8egWgo18nHoGz8jpn7qKetn+q9Dq+H/JMs5e/OcJtSqum+hgGmvatHeDn64wegf KGNsiHqMGN0BclqsKlruA9fBoL/WvxwAViDNifddupUw2E2id6MGMGHC1nMYWx9HPybStRnQ5xX6r HIdFtTJu3FqoU84PQaEMx39fKv8sGL7Q5TP3FzmwIObRUUFi8GHVYnPzTcsl6C4ZLyqqUn7gh5e/1 89LbSWCQ6oqQO7pnFdzg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lpcfG-00H26R-Mz; Sat, 05 Jun 2021 20:13:02 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lpcfD-00H263-0z for linux-arm-kernel@lists.infradead.org; Sat, 05 Jun 2021 20:13:00 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 4DBA86120E; Sat, 5 Jun 2021 20:12:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1622923975; bh=oiClRQ8zNAbJxP7fr7sthVt6TG4eCBIs03ldx9MHKTU=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=pHygaBTofx5Zsas0gkUSSpCT0vgcTPSrpT6Fh1j1q/hLCci+7T7jPo2Tq+UNwXfZP zypnr1hvpJ7Msl0qOpvIFZ4iVVq8lWtitKrV4Y/0QY4V8ZWZuNTYy4Tyh1cGXTNUAj wanvE40shXxxW/ERojHgWuO7XtukY2L9lbwi51AN3HBQRVleXSZKFqCC3gF1Dg63ze Mtovy7QZS8DngLJZ5JK7kCbWsz7OI3f21IVdrlJ4hmTytgUUQJv+gRK3IEcvUK8BiD wXKFkM88scyhwOdT6K3K379+54FFEEutJp0VESbDo9RYRSt7yZgNrh1A+haLt7nbGZ u1P/+0gGqTcTg== Date: Sat, 5 Jun 2021 15:12:53 -0500 From: Bjorn Helgaas To: Sandor Bodo-Merle Cc: Ray Jui , Scott Branden , bcm-kernel-feedback-list@broadcom.com, Bjorn Helgaas , Pali =?iso-8859-1?Q?Roh=E1r?= , Ray Jui , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Lorenzo Pieralisi , linux-pci@vger.kernel.org Subject: Re: [PATCH] PCI: iproc: restrict multi-MSI to single core CPUs Message-ID: <20210605201253.GA2318292@bjorn-Precision-5520> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210605171736.15755-1-sbodomerle@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210605_131259_130547_12369C13 X-CRM114-Status: GOOD ( 27.62 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org [+cc Lorenzo, linux-pci] You can use this to find the appropriate cc list: ./scripts/get_maintainer.pl -f drivers/pci/controller/pcie-iproc-msi.c I added Lorenzo and linux-pci for you. Please update the subject line to: PCI: iproc: Support multi-MSI only on uniprocessor kernel On Sat, Jun 05, 2021 at 07:17:36PM +0200, Sandor Bodo-Merle wrote: > Commit fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs") > introduced multi-MSI support with a broken allocation mechanism (it faile= d to > reserve the proper number of bits from the inner domain). Natural alignm= ent of > the base vector number was also not guaranteed. This sounds like it's fixing *two* problems: the bitmap allocation problem above, and the multi-MSI restriction problem below. Please split this into two separate patches if possible. > The interrupt affinity scheme used by this driver is incompatible with > multi-MSI as implies moving the doorbell address to that of another MSI g= roup. > This isn't possible for Multi-MSI, as all the MSIs must have the same doo= rbell > address. As such it is restricted to systems with single CPU core. Please rewrap the commit log to fit in 75 columns, so it still fits in 80 when "git log" indents it. s/as implies/as it implies/ s/Multi-MSI/multi-MSI/ (or capitalize them all; just be consistent) s/with single CPU core/with a single CPU/ Using "core" here ("single core CPUs" or "single CPU core") suggests that this has something to do with single-core CPUs vs multi-core CPUs, but I don't think that's the case. The patch says the important thing is whether the kernel supports one CPU or several CPUs. Whether they're in a single package or not is irrelevant. And apparently multi-MSI even works fine when you boot a uniprocessor kernel (CONFIG_NR_CPUS=3D1) on a multi-processor machine. > Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs") > Reported-by: Pali Roh=E1r > Signed-off-by: Sandor Bodo-Merle > --- > drivers/pci/controller/pcie-iproc-msi.c | 23 ++++++++++++----------- > 1 file changed, 12 insertions(+), 11 deletions(-) > = > diff --git drivers/pci/controller/pcie-iproc-msi.c drivers/pci/controller= /pcie-iproc-msi.c Patch is incorrectly generated and lacks a path element, so doesn't apply cleanly. I don't know how you did this, but it should look like this (note the leading "a/" and "b/"): diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/contro= ller/pcie-iproc-msi.c > index eede4e8f3f75..2e42c460b626 100644 > --- drivers/pci/controller/pcie-iproc-msi.c > +++ drivers/pci/controller/pcie-iproc-msi.c > @@ -171,7 +171,7 @@ static struct irq_chip iproc_msi_irq_chip =3D { > = > static struct msi_domain_info iproc_msi_domain_info =3D { > .flags =3D MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | > - MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX, > + MSI_FLAG_PCI_MSIX, > .chip =3D &iproc_msi_irq_chip, > }; > = > @@ -252,18 +252,15 @@ static int iproc_msi_irq_domain_alloc(struct irq_do= main *domain, > = > mutex_lock(&msi->bitmap_lock); > = > - /* Allocate 'nr_cpus' number of MSI vectors each time */ > - hwirq =3D bitmap_find_next_zero_area(msi->bitmap, msi->nr_msi_vecs, 0, > - msi->nr_cpus, 0); > - if (hwirq < msi->nr_msi_vecs) { > - bitmap_set(msi->bitmap, hwirq, msi->nr_cpus); > - } else { > - mutex_unlock(&msi->bitmap_lock); > - return -ENOSPC; > - } > + /* Allocate 'nr_irqs' multiplied by 'nr_cpus' number of MSI vectors eac= h time */ Can you wrap this comment so it fits in 80 columns, please? The rest of the file is formatted for 80 columns, so it will be nice if this matches. > + hwirq =3D bitmap_find_free_region(msi->bitmap, msi->nr_msi_vecs, > + order_base_2(msi->nr_cpus * nr_irqs)); > = > mutex_unlock(&msi->bitmap_lock); > = > + if (hwirq < 0) > + return -ENOSPC; > + > for (i =3D 0; i < nr_irqs; i++) { > irq_domain_set_info(domain, virq + i, hwirq + i, > &iproc_msi_bottom_irq_chip, > @@ -284,7 +281,8 @@ static void iproc_msi_irq_domain_free(struct irq_doma= in *domain, > mutex_lock(&msi->bitmap_lock); > = > hwirq =3D hwirq_to_canonical_hwirq(msi, data->hwirq); > - bitmap_clear(msi->bitmap, hwirq, msi->nr_cpus); > + bitmap_release_region(msi->bitmap, hwirq, > + order_base_2(msi->nr_cpus * nr_irqs)); > = > mutex_unlock(&msi->bitmap_lock); > = > @@ -539,6 +537,9 @@ int iproc_msi_init(struct iproc_pcie *pcie, struct de= vice_node *node) > mutex_init(&msi->bitmap_lock); > msi->nr_cpus =3D num_possible_cpus(); > = > + if (msi->nr_cpus =3D=3D 1) > + iproc_msi_domain_info.flags |=3D MSI_FLAG_MULTI_PCI_MSI; > + > msi->nr_irqs =3D of_irq_count(node); > if (!msi->nr_irqs) { > dev_err(pcie->dev, "found no MSI GIC interrupt\n"); > -- = > 2.31.0 > = _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel