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 547CAC4332F for ; Thu, 8 Dec 2022 19:02:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=+PxJlFNfhVWQYFZswMinWLO/Cqlj7sG+G9SJzoq8UKU=; b=yyNKH/MZ0skFTX us+hZx3tM+WwaK0J7kd3iPX5ytMLdvGmqpterAQ87DCKP70kCFfWculJb+vgGgZ0u8g0Uu/Pn89tP jVnDSI6rRHIonhDhk9j+xQz37fQQVZc8kVzHTCtqi6XMeSzzsHRhpjGL+KfSl+Z/rah/di4yzkgNA rIg8rpCGdnD+Gwbki5VDfqx6oxmgt0QgmBQKis7b0lbJxO83n7cQcZsyH8IJpxEFf5QhZI/DmjCfR cQGW0gVk028NfbDYfhpbNcPWD0nEiWj6Acanxr0jzngzgjucLE1xhUGQXEqVgd69SuWJQgMryDKvt yqi7HkCW0fNt5BKEpzAA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p3M9G-008xPH-2S; Thu, 08 Dec 2022 19:01:34 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1p3M9C-008xFN-8w for linux-arm-kernel@lists.infradead.org; Thu, 08 Dec 2022 19:01:32 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3A1B923A; Thu, 8 Dec 2022 11:01:29 -0800 (PST) Received: from [10.1.196.40] (e121345-lin.cambridge.arm.com [10.1.196.40]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0C5793F73D; Thu, 8 Dec 2022 11:01:20 -0800 (PST) Message-ID: <2a188b8a-ab16-d5d4-ed5f-f8eec236e4ca@arm.com> Date: Thu, 8 Dec 2022 19:01:16 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [RFC PATCH] arm64: dts: ls1028a: mark ARM SMMU as DMA coherent Content-Language: en-GB To: Vladimir Oltean , devicetree@vger.kernel.org, iommu@lists.linux.dev Cc: Laurentiu Tudor , Will Deacon , linux-arm-kernel@lists.infradead.org, Shawn Guo , Li Yang , Rob Herring , Krzysztof Kozlowski , linux-kernel@vger.kernel.org, Michael Walle References: <20221208151514.3840720-1-vladimir.oltean@nxp.com> From: Robin Murphy In-Reply-To: <20221208151514.3840720-1-vladimir.oltean@nxp.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221208_110130_454223_BC37FFC5 X-CRM114-Status: GOOD ( 25.97 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 08/12/2022 3:15 pm, Vladimir Oltean wrote: > Since commit df198b37e72c ("iommu/arm-smmu: Report > IOMMU_CAP_CACHE_COHERENCY better"), the SMMU driver will say that a > device has the IOMMU_CAP_CACHE_COHERENCY capability if the > ARM_SMMU_FEAT_COHERENT_WALK bit was set in smmu->features. > > This breaks vfio-pci, as can be seen below: > > $ echo 0000:00:00.0 > /sys/bus/pci/drivers/fsl_enetc/unbind > $ echo vfio-pci > /sys/bus/pci/devices/0000\:00\:00.0/driver_override > $ echo 0000:00:00.0 > /sys/bus/pci/drivers/vfio-pci/bind > [ 25.261941] vfio-pci 0000:00:00.0: arm_smmu_capable: smmu features 0xe9e > [ 25.268877] vfio-pci 0000:00:00.0: vfio_group_find_or_alloc: device_iommu_capable() returned false > [ 25.279271] vfio-pci 0000:00:00.0: vfio_pci_core_register_device: failed to register group dev: -EINVAL > [ 25.301377] vfio-pci: probe of 0000:00:00.0 failed with error -22 > > The ARM_SMMU_FEAT_COHERENT_WALK feature is set in > arm_smmu_device_dt_probe() if the OF node of the SMMU device was set as > dma-coherent. In the case of LS1028A, it wasn't. > > Fix that. > > Signed-off-by: Vladimir Oltean > --- > The LS1028A is not the only SoC affected by this, it seems. > fsl-ls1088a.dtsi seems to be in the same situation where vfio-pci worked > before. There are also other SoCs which don't have dma-coherent in the > iommu node. There's also something I don't quite like about this patch > technically introducing a regression which requires a device tree update. > > Can something different be done about that, or are LS1028A/LS1088A > simply to blame because of breaching the dt-bindings contract, and in > that case, I'll have to suck it up, put a Fixes tag here, write another > patch for LS1088A, and resend? It's more just good fortune that it ever worked properly at all. We have to make the DT authoritative about coherency because cases exist where the ID register is misconfigured. You've been telling Linux that that is the case, and now the message is finally getting through to VFIO. If we weren't also lazy in io-pgtable-arm about what shareability attribute to use for IOMMU_CACHE, you would have actually had the broken VFIO behaviour that that check is now defending against. I'd argue that you do want to make the DT change, because it's the truth of the hardware. Even if you did want to keep doing the significant extra work of maintaining non-coherent pagetables (there is a dubious snoop latency vs. TLB miss rate argument), that would be better achieved at the level of the io_pgtable_cfg, not by lying about the entire SMMU. However, since Jason refactored things at the VFIO end too, it looks like this should now be consistently checked for every individual device bound to a VFIO driver, so we might be able to do a bit better, as below. I'd be rather surprised if anyone ever genuinely built this topology, but it does happen to be the one other combination that's easy to infer with reasonable confidence. Thanks, Robin. ----->8----- diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 30dab1418e3f..a5ad9d6b51cf 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1320,7 +1320,8 @@ static bool arm_smmu_capable(struct device *dev, enum iommu_cap cap) switch (cap) { case IOMMU_CAP_CACHE_COHERENCY: /* Assume that a coherent TCU implies coherent TBUs */ - return cfg->smmu->features & ARM_SMMU_FEAT_COHERENT_WALK; + return cfg->smmu->features & ARM_SMMU_FEAT_COHERENT_WALK || + device_get_dma_attr(dev) == DEV_DMA_COHERENT; case IOMMU_CAP_NOEXEC: return true; default: _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel