From patchwork Tue Apr 8 18:08:28 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Gunthorpe X-Patchwork-Id: 3950371 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2151CBFF02 for ; Tue, 8 Apr 2014 18:09:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 43CDE20221 for ; Tue, 8 Apr 2014 18:09:33 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2BE39201C7 for ; Tue, 8 Apr 2014 18:09:32 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WXaSD-0001bw-MT; Tue, 08 Apr 2014 18:09:01 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WXaS9-0004cc-Ti; Tue, 08 Apr 2014 18:08:58 +0000 Received: from quartz.orcorp.ca ([184.70.90.242]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WXaS5-0004au-UT for linux-arm-kernel@lists.infradead.org; Tue, 08 Apr 2014 18:08:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=obsidianresearch.com; s=rsa1; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=OTgucWOk4emSBI/SMunwKEqNyBJ18ye/Jnw6jqGDZF4=; b=zVg0cvXo/oQv1Y0JIhUPAcFlsLLWwBaRli42SAKcNYUyHP1nywLoNJqDN11hcp4RE1jG4Aaku0B8PQ6x7XUbYBaTRh8CQmcI4yPtzasvTYSSJerw4VPpcUVpOS7DK4qYNfdVG89+XL3yDKUq5zJ0Z86OTEOCmdaTVy2j3CG5bGk=; Received: from [10.0.0.161] (helo=jggl.edm.orcorp.ca) by quartz.orcorp.ca with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1WXaRh-000555-0s; Tue, 08 Apr 2014 12:08:29 -0600 Received: from jgg by jggl.edm.orcorp.ca with local (Exim 4.80) (envelope-from ) id 1WXaRg-0002hs-MS; Tue, 08 Apr 2014 12:08:28 -0600 Date: Tue, 8 Apr 2014 12:08:28 -0600 From: Jason Gunthorpe To: Willy Tarreau Subject: Re: Intel I350 mini-PCIe card (igb) on Mirabox (mvebu / Armada 370) Message-ID: <20140408180828.GC32490@obsidianresearch.com> References: <20140405193435.50d8dd81@skate> <54BB31A2B04145E8908E0183FAB6B61B@fatboyfat.co.uk> <20140408171309.09bbf968@skate> <20140408174034.79df403e@skate> <20140408175545.1b4d55a5@skate> <20140408171417.GB27776@obsidianresearch.com> <20140408175324.GH11052@1wt.eu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140408175324.GH11052@1wt.eu> User-Agent: Mutt/1.5.21 (2010-09-15) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.161 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140408_140854_093673_6D3799CC X-CRM114-Status: GOOD ( 24.69 ) X-Spam-Score: -2.0 (--) Cc: Thomas Petazzoni , Neil Greatorex , Matthew Minter , linux-arm-kernel X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Apr 08, 2014 at 07:53:24PM +0200, Willy Tarreau wrote: > Hi Jason, > > On Tue, Apr 08, 2014 at 11:14:17AM -0600, Jason Gunthorpe wrote: > > The mbus hardware requires a power of two size, if a non-power > > of two is passed in to the low level routines they configure the > > register in a way that results in undefined behaviour. > > > > - WARN_ON to make this invalid usage really obvious > > - Round down to the nearest power of two so we only stuff a valid > > size into the HW > > So if I understand it right, for example when my first NIC registers > e0000000-e02fffff, then we'll truncate it down to e0000000-e01fffff, > won't that cause any issue, for example if the NIC needs to access > data beyond that limit ? The mbus windows are assigned to the bridge, not to the NIC bars: 00:0a.0 PCI bridge [0604]: Marvell Technology Group Ltd. Device [11ab:7846] (rev 02) (prog-if 00 [Normal decode]) Memory behind bridge: e0000000-e17fffff So the above is not OK, 17fffff is not a power of two. The patch causes the mbus to WARN_ON and then round down so that the hardware is at least in a defined configuration. > BTW I can try your patch with the myricom NIC which started to work > when rounding up, I'll quickly see if that fixes the issue as well, > but I'm now a little bit confused. This patch won't fix anything, it just fixes the mbus driver to always program the hardware with valid values, even if the upper layer requests something invalid. Basically, we spent a few weeks tracking this behavior down, the patch would ensure that time doesn't get spent again. The goal now is to avoid the WARN_ON in the patch from firing. For a proper fix, something like this to create multiple aligned windows is a simple option (untested, need the inverse on the de-register, loop should probably live in mbus code): Jason diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index 789cdb2..7312c82 100644 --- a/drivers/pci/host/pci-mvebu.c +++ b/drivers/pci/host/pci-mvebu.c @@ -382,6 +382,9 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port) static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) { + phys_addr_t base; + unsigned int mapped_size; + /* Are the new membase/memlimit values invalid? */ if (port->bridge.memlimit < port->bridge.membase || !(port->bridge.command & PCI_COMMAND_MEMORY)) { @@ -408,8 +411,16 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port) (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) - port->memwin_base; - mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr, - port->memwin_base, port->memwin_size); + base = port->memwin_base; + mapped_size = 0; + while (mapped_size < port->memwin_size) { + unsigned int size = + rounddown_pow_of_two(port->memwin_size - mapped_size); + mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr, + base, size); + base += size; + mapped_size += size; + } }