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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 71045C87FCA for ; Thu, 7 Aug 2025 17:23:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=auGb5g7V45LMPUrtbY6e4g8GFTxl+FOOZLNMba2GOuQ=; b=lew7qkw4XzAr28rZm77yMflo3e mDkPkBayFnIz8hJf74fbdcN6UJgj5F4MLy/XHMBnwCua7pIJpuKEyAX1q+NxWcdegDXN5+pIWAFUX idkwiQA27JI4XRRzXB4UmFfoY/9dFfQLoHnzUQHXnmgcaMX1uhym6ciniaw4Je8z9EyInLwjAyett QzDS8qDWBLz0vH8XVQ66szC47qfl3+Vu2AjLdhrARdpA/1H3TsZz00QbChmFo3diYqxx5R/Z2tlEw zSGgg/udFc0DhP3Vg3bsMhngoU5joWJIS49HW0hZX8O8colTTSTBtIeckehWaZ6D4a5YPOAHJP2OW ewwd5tAg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uk4Ks-00000001Ez4-2IYe; Thu, 07 Aug 2025 17:23:26 +0000 Received: from nyc.source.kernel.org ([2604:1380:45d1:ec00::3]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uk4IK-00000001Epj-2rha for linux-arm-kernel@lists.infradead.org; Thu, 07 Aug 2025 17:20:49 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 965D1A5642C; Thu, 7 Aug 2025 17:20:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4135AC4CEED; Thu, 7 Aug 2025 17:20:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1754587247; bh=e2RyNcZjMIJHuZUIhTCHbHcsfF9RgB8S/NxnIlLipao=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=WA0tPp+8mfbmRWW0wF6J067wl2sj+05XIDGHPMpm3Qa1UJcHkJXqUDcLOSvgXU3t2 nJZm3Sb2OUQOX33P2Uv1D5/+vk1gZSuDpeI28o0lYIUFoIqOpRMXfo5Rynp2C5z+Je HtGDymzlBvUFok61JP3tL6EZgkPtvsypLWSbcvfw4wsFyDfJEpM14Bu2M3OvYYUEgg RS2/LDVDQ/NgnBZ4P5gbstkonqBTZ0bHplDiKKDX6WyJ3B8pI+UyUOcBAm5CQf1U4w jeD0st2RKwwWXLhf5qGi1ePTyk2B5PM/eR5IK3e2RFO51VQO5tgcDsvj6JNYSClDy+ udRSp/gtDBkhg== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1uk4IH-0050gp-0r; Thu, 07 Aug 2025 18:20:45 +0100 Date: Thu, 07 Aug 2025 18:20:44 +0100 Message-ID: <8634a39hg3.wl-maz@kernel.org> From: Marc Zyngier To: Christian Bruel Cc: , , , , Subject: Re: [PATCH] irqchip: gic-v2m: Handle multiple MSI base IRQ mis-alignment In-Reply-To: <20250807114759.3966195-1-christian.bruel@foss.st.com> References: <20250807114759.3966195-1-christian.bruel@foss.st.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: christian.bruel@foss.st.com, tglx@linutronix.de, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, fabrice.gasnier@foss.st.com, mani@kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250807_102048_848023_282138BF X-CRM114-Status: GOOD ( 30.06 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 07 Aug 2025 12:47:58 +0100, Christian Bruel wrote: > > The PCI Local Bus Specification (section 6.8.3.4 in Rev 3) allows modifying > the low-order bits of the MSI Message Data to encode the interrupt number. > However, the base SPI used for allocation may not be aligned to the > requested number of irqs. > > For instance, on STM32MP25 with an initial MSI_TYPER base SPI of 0x16A, > allocating a multiple MSI of size 8 with the first two slots reserved, the > offset returned is 8, resulting in an MSI DATA base of 0x172. > This causes the endpoint device to send a wrong interrupt number: > > 1st MSI = 0x172 | 0x0 = 0x172 > 2nd MSI = 0x172 | 0x1 = 0x173 > 3rd MSI = 0x172 | 0x2 = 0x172 wrongly triggers the 1st MSI > > The alignment offset can be computed in the gicv2m driver: > replacing bitmap_find_free_region() with bitmap_find_next_zero_area_off() > to accommodate the required alignment. Well, that's only telling half of the story. We get there because the SPI range has been allocated in a pretty dumb manner (the starting SPI is not a multiple of 32). Pretty damning if you're considering PCI. > > Without this fix, the workaround is to force the alignment in the DT > within the MSI range (if 32 MSIs are mapped from 362 to 393): > arm,msi-base-spi = <368> > arm,msi-num-spis = <26> > with the effect of reducing the number of available MSIs. This doesn't really belong here. But the example is pretty wrong anyway, because what you need for PCI is the low 5 bits to be clear, so that you can allocate 32 MSIs. So your range would start at 384, and... oh wait... > > Change-Id: I316a580755cd1b1684929d2540295f4a45f0532d Neither does this. > Signed-off-by: Christian Bruel > --- > drivers/irqchip/irq-gic-v2m.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c > index 24ef5af569fe..21a14d15e7a9 100644 > --- a/drivers/irqchip/irq-gic-v2m.c > +++ b/drivers/irqchip/irq-gic-v2m.c > @@ -153,14 +153,18 @@ static int gicv2m_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, > { > msi_alloc_info_t *info = args; > struct v2m_data *v2m = NULL, *tmp; > - int hwirq, offset, i, err = 0; > + int hwirq, i, err = 0; > + unsigned long align_mask = (1 << get_count_order(nr_irqs)) - 1; Use BIT(). > + unsigned long align_off, offset; > > spin_lock(&v2m_lock); > list_for_each_entry(tmp, &v2m_nodes, entry) { > - offset = bitmap_find_free_region(tmp->bm, tmp->nr_spis, > - get_count_order(nr_irqs)); > - if (offset >= 0) { > + align_off = tmp->spi_start & info->desc->pci.msi_attrib.multiple; Err, no. MSIs are not just PCI only, and there is no reason why you should go and rummage into these data structures. That's none of an irqchip's business. The correct way to go about this is to consider nr_irqs, because that's what you are trying to align for. But I really don't get how you compute that offset. 'multiple' is the number of bits required to encode the number of vectors. How does this work? > + offset = bitmap_find_next_zero_area_off(tmp->bm, tmp->nr_spis, 0, > + nr_irqs, align_mask, align_off); > + if (offset < tmp->nr_spis) { > v2m = tmp; > + bitmap_set(v2m->bm, offset, nr_irqs); > break; > } > } M. -- Without deviation from the norm, progress is not possible.