Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp2113191pxj; Sat, 5 Jun 2021 13:17:36 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxSreIDBzs7GSQEgLXAuRIEJoan5aMgjEy2VsGVP5VgPOMoHEhdRw85NQ4vBu6prFWqqjvx X-Received: by 2002:a17:907:2057:: with SMTP id pg23mr10307615ejb.113.1622924255998; Sat, 05 Jun 2021 13:17:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1622924255; cv=none; d=google.com; s=arc-20160816; b=BElQ/VspwBEPAhmIys2rUB6ur5+Ata6ptofFFEOg3D8VFEGWc5YDEutZkJl0Q5nHbG 7yqGxTmX+YBA7oU0pMMaWAODHsqedS4IBTinFj1cJIuMN5Gv6vZgRKV5D76XJslW2nS9 ErecVugZCCeXiWvehj5ulP4TYCI+Mez9eCDIfYMl76qf86XtidMOgh2Nqlr7z42x6Ssz FuKx7yOYIqBKKEwcKBYZNH9vZA+nQyzC/6PN6zk+gri7my4krKZ17dRQ2zIs3pVl+O3z 32gmOIRr1BPwfSiD0SZGWZ9SzBO2jltR0h3AIlVV/7stnAxtxwN+fl09PFcPKz1xuIgy EVbQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:message-id:subject:cc:to:from:date :dkim-signature; bh=eP7sk0YaIkbOYpHWMlq68Uua1Z5zZuCXjV2AWb3PPUQ=; b=Kd6ePLjJoIQV9Bx5zpR6yz9kWN/CDRa0xWcuRSxVJ0gpb8w1lDXi6KbPxJr5KHNd++ yKrC0VjkVKzIh59gRU/7dxUKXWx3LMitHH+xslcWyudP+eI6p+EG+1AAqD1N/ka8hEqU mQpadtPXUzOhpf4WD5qzQqG7x3gtyI2pBHvuHiZ8C6Rw446JumSq7e0A+jRcAiImuAlL EJWXDg30k1E0197yUPg1vti9630iCkRg+LuQdFS5Nu3QcPKCkk1PPBG4aBc+Sd8OtaLu qm6CIO1aiWqo68PMa0wYUcS3ANfOOdSGhSueynqx77910QCzswzaTDzmY1SzybMfnMhb kCRg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=pHygaBTo; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id z62si9701989ede.508.2021.06.05.13.17.12; Sat, 05 Jun 2021 13:17:35 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=pHygaBTo; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230029AbhFEUOp (ORCPT + 99 others); Sat, 5 Jun 2021 16:14:45 -0400 Received: from mail.kernel.org ([198.145.29.99]:56300 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229998AbhFEUOo (ORCPT ); Sat, 5 Jun 2021 16:14:44 -0400 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-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20210605171736.15755-1-sbodomerle@gmail.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.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 failed to > reserve the proper number of bits from the inner domain). Natural alignment 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 group. > This isn't possible for Multi-MSI, as all the MSIs must have the same doorbell > 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=1) on a multi-processor machine. > Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs") > Reported-by: Pali Roh?r > 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/controller/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 = { > > static struct msi_domain_info iproc_msi_domain_info = { > .flags = 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 = &iproc_msi_irq_chip, > }; > > @@ -252,18 +252,15 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain, > > mutex_lock(&msi->bitmap_lock); > > - /* Allocate 'nr_cpus' number of MSI vectors each time */ > - hwirq = 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 each 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 = 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 = 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_domain *domain, > mutex_lock(&msi->bitmap_lock); > > hwirq = 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 device_node *node) > mutex_init(&msi->bitmap_lock); > msi->nr_cpus = num_possible_cpus(); > > + if (msi->nr_cpus == 1) > + iproc_msi_domain_info.flags |= MSI_FLAG_MULTI_PCI_MSI; > + > msi->nr_irqs = of_irq_count(node); > if (!msi->nr_irqs) { > dev_err(pcie->dev, "found no MSI GIC interrupt\n"); > -- > 2.31.0 >