From patchwork Mon Mar 27 17:33:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lorenzo Pieralisi X-Patchwork-Id: 9647213 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 84BD9602C8 for ; Mon, 27 Mar 2017 17:33:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6A1EB27F85 for ; Mon, 27 Mar 2017 17:33:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5D1BF283FF; Mon, 27 Mar 2017 17:33:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E16DB27F85 for ; Mon, 27 Mar 2017 17:33:24 +0000 (UTC) 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:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=OYeNAJuTQa4ESScT04ZFkeEkqueoBfi1/msHYu1NgGI=; b=UOvYdUQJcZFyrU DwsunOSKb5tXYjoQ6jrquzW5z31NOVjURIJ5bfLjyDG2Cf4J10Ufl/akS22lwvszBDfQDNzbJayiK Fn4AKS9bETyjLxq7YHLS1O6DtHtpmDoRV1K51ErpQTz6P3+akKRxXrtrQZQYC2Usn0y55dn2h3PHF x7pVLxFmMeS5kX7u57l9RVPt5Bl6BG5drIuNkeO+ZzTyO30CFt/PSVtBU1sVRdaq1o6U9JOzBCL3l Dl4xGSVdmHS8yC9qA6llp4gtrWiptJM6UU+PW4MwENu3Z8sQvlxQamn5csGWn0RCM6kizd+00Ibkh 2L3EiEA0enSpoTAwWzWg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1csYW5-0000Cq-3k; Mon, 27 Mar 2017 17:33:17 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1csYW0-0000Bu-Qz for linux-arm-kernel@lists.infradead.org; Mon, 27 Mar 2017 17:33:14 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 004B928; Mon, 27 Mar 2017 10:32:52 -0700 (PDT) Received: from red-moon (red-moon.cambridge.arm.com [10.1.206.55]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 69B173F23B; Mon, 27 Mar 2017 10:32:49 -0700 (PDT) Date: Mon, 27 Mar 2017 18:33:14 +0100 From: Lorenzo Pieralisi To: Robin Murphy Subject: Re: [PATCH V9 00/11] IOMMU probe deferral support Message-ID: <20170327173314.GA17035@red-moon> References: <1489086061-9356-1-git-send-email-sricharan@codeaurora.org> <58D49845.9060407@hisilicon.com> <0ea8022b-a19b-335d-6cc6-81510196f891@codeaurora.org> <5FC3163CFD30C246ABAA99954A238FA81750B7CB@lhreml504-mbs> <5FC3163CFD30C246ABAA99954A238FA81750CCC4@lhreml504-mbs> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170327_103312_901366_36BA8DBF X-CRM114-Status: GOOD ( 22.24 ) 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: "linux-arm-msm@vger.kernel.org" , "joro@8bytes.org" , "will.deacon@arm.com" , "iommu@lists.linux-foundation.org" , Shameerali Kolothum Thodi , "okaya@codeaurora.org" , "linux-acpi@vger.kernel.org" , "Wangzhou \(B\)" , "hanjun.guo@linaro.org" , "linux-pci@vger.kernel.org" , "bhelgaas@google.com" , "tn@semihalf.com" , Sricharan R , "linux-arm-kernel@lists.infradead.org" , "m.szyprowski@samsung.com" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Mar 27, 2017 at 05:18:15PM +0100, Robin Murphy wrote: [...] > >> [ 145.212351] iommu: Adding device 0000:81:10.0 to group 5 > >> [ 145.212367] ixgbevf 0000:81:10.0: 0x0 0x100000000, 0x0 0xffffffffffff, > >> 0xffffffff 0xffffffff > >> [ 145.213261] ixgbevf 0000:81:10.0: enabling device (0000 -> 0002) > >> [ 145.213394] ixgbe 0000:81:00.0 eth0: VF Reset msg received from vf 0 > >> [ 145.214272] ixgbe 0000:81:00.0: VF 0 has no MAC address assigned, you > >> may have to assign one manually > >> [ 145.224379] ixgbevf 0000:81:10.0: MAC address not assigned by > >> administrator. > >> [ 145.224384] ixgbevf 0000:81:10.0: Assigning random MAC address > >> [ 145.225941] ixgbevf 0000:81:10.0: 1a:85:06:48:a7:19 > >> [ 145.225944] ixgbevf 0000:81:10.0: MAC: 1 > >> [ 145.225946] ixgbevf 0000:81:10.0: Intel(R) 82599 Virtual Function > >> [ 145.299961] ixgbe 0000:81:00.0 eth0: NIC Link is Up 1 Gbps, Flow Control: > >> None > >> [ 154.947742] nfs: server 172.18.45.166 not responding, still trying > >> [ 191.025780] nfs: server 172.18.45.166 not responding, still trying > >> [ 191.026122] nfs: server 172.18.45.166 OK > >> [ 191.026317] nfs: server 172.18.45.166 OK > >> [ 263.706402] VFIO - User Level meta-driver version: 0.3 > >> [ 269.757613] vfio-pci 0000:81:10.0: 0x0 0x0, 0x0 0xffffffffffff, 0xffffffffffffffff > >> 0xffffffffffffffff > >> [ 269.757617] specified DMA range outside IOMMU capability > >> [ 269.757618] Failed to set up IOMMU for device 0000:81:10.0; retaining > >> platform DMA ops > >> > >> From the logs its clear that when ixgbevf driver originally probes and adds > >> the device > >> to smmu the dma mask is 32, but when it binds to vfio-pci, it becomes 64 bit. > > > > Just to add to that, the mask is set to 64 bit in the ixgebvf driver probe[1] > > Aha, but of course it's still the same struct device getting bound to > VFIO later, so whatever mask the first driver set is still in there when > we go through of_dma_configure() the second time (and the fact that we > go through more than once being the new behaviour). So yes, this is a > legitimate problem and we really do need to be robust against size > overflow. I reckon the below tweak of your fix is probably the way to > go; cleaning up the arch_setup_dma_ops() interface can happen later. Yes. On ACPI the additional issue is, given that we now call *_dma_configure multiple times, we end up adding the _same_ stream id to an existing fwspec ids array which triggers the SMMUv3 driver bug, because we try to configure the same id twice (it does not happen in DT since you have a check similar to the one I added below). ACPI fix(es) below please let me know if it actually fixes the issues: -- >8 -- diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index 36a9abf..e7b1940 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -543,6 +543,14 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev, const struct iommu_ops *ops = NULL; int ret = -ENODEV; struct fwnode_handle *iort_fwnode; + struct iommu_fwspec *fwspec = dev->iommu_fwspec; + + /* + * If we already translated the fwspec there + * is nothing left to do, return the iommu_ops. + */ + if (fwspec && fwspec->ops) + return fwspec->ops; if (node) { iort_fwnode = iort_get_fwnode(node); diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 823b005..2a513cc 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1376,18 +1376,20 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev) int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) { const struct iommu_ops *iommu; + u64 size; iort_set_dma_mask(dev); iommu = iort_iommu_configure(dev); if (IS_ERR(iommu)) return PTR_ERR(iommu); + + size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1); /* * Assume dma valid range starts at 0 and covers the whole * coherent_dma_mask. */ - arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu, - attr == DEV_DMA_COHERENT); + arch_setup_dma_ops(dev, 0, size, iommu, attr == DEV_DMA_COHERENT); return 0; }