From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Gonzalez Subject: Re: [PATCH] arm64/io: Don't use WZR in writel Date: Tue, 12 Mar 2019 13:36:25 +0100 Message-ID: <33d765b5-1807-fa6c-1ceb-99f09f7c8d5a@free.fr> References: <68b71c15f32341468a868f6418e4fcb375bc49ba.camel@gmail.com> <20190211105755.GB30880@fuggles.cambridge.arm.com> <38d8965a-cd41-17cf-1b95-8dd58c079be4@arm.com> <874c702b8af760aa8fae38d478c79e3ecba00515.camel@gmail.com> <235d20ef-3054-69d9-975d-25aebf32aad3@arm.com> <20190223181254.GC572@tuxbook-pro> <86zhqm8i6d.wl-marc.zyngier@arm.com> <20190224035356.GD572@tuxbook-pro> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190224035356.GD572@tuxbook-pro> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Marc Zyngier , Will Deacon , Robin Murphy Cc: Bjorn Andersson , AngeloGioacchino Del Regno , Jens Axboe , Catalin Marinas , LKML , Linux ARM , MSM , Jeffrey Hugo List-Id: linux-arm-msm@vger.kernel.org On 24/02/2019 04:53, Bjorn Andersson wrote: > On Sat 23 Feb 10:37 PST 2019, Marc Zyngier wrote: > >> On Sat, 23 Feb 2019 18:12:54 +0000, Bjorn Andersson wrote: >>> >>> On Mon 11 Feb 06:59 PST 2019, Marc Zyngier wrote: >>> >>>> On 11/02/2019 14:29, AngeloGioacchino Del Regno wrote: >>>> >>>>> Also, just one more thing: yes this thing is going ARM64-wide and >>>>> - from my findings - it's targeting certain Qualcomm SoCs, but... >>>>> I'm not sure that only QC is affected by that, others may as well >>>>> have the same stupid bug. >>>> >>>> At the moment, only QC SoCs seem to be affected, probably because >>>> everyone else has debugged their hypervisor (or most likely doesn't >>>> bother with shipping one). >>>> >>>> In all honesty, we need some information from QC here: which SoCs are >>>> affected, what is the exact nature of the bug, can it be triggered from >>>> EL0. Randomly papering over symptoms is not something I really like >>>> doing, and is likely to generate problems on unaffected systems. >>> >>> The bug at hand is that the XZR is not deemed a valid source in the >>> virtualization of the SMMU registers. It was identified and fixed for >>> all platforms that are shipping kernels based on v4.9 or later. >> >> When you say "fixed": Do you mean fixed in the firmware? Or by adding >> a workaround in the shipped kernel? > > I mean that it's fixed in the firmware. > >> If the former, is this part of an official QC statement, with an >> associated erratum number? > > I don't know, will get back to you on this one. > >> Is this really limited to the SMMU accesses? > > Yes. > >>> As such Angelo's list of affected platforms covers the high-profile >>> ones. In particular MSM8996 and MSM8998 is getting pretty good support >>> upstream, if we can figure out a way around this issue. >> >> We'd need an exhaustive list of the affected SoCs, and work out if we >> can limit the hack to the SMMU driver (cc'ing Robin, who's the one >> who'd know about it). > > I will try to compose a list. FWIW, I have just been bitten by this issue. I needed to enable an SMMU to filter PCIe EP accesses to system RAM (or something). I'm using an APQ8098 MEDIABOX dev board. My system hangs in arm_smmu_device_reset() doing: /* Invalidate the TLB, just in case */ writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH); writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH); With the 'Z' constraint, gcc generates: str wzr, [x0] without the 'Z' constraint, gcc generates: mov w1, 0 str w1, [x0] I can work around the problem using the following patch: diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 045d93884164..93117519aed8 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -59,6 +59,11 @@ #include "arm-smmu-regs.h" +static inline void qcom_writel(u32 val, volatile void __iomem *addr) +{ + asm volatile("str %w0, [%1]" : : "r" (val), "r" (addr)); +} + #define ARM_MMU500_ACTLR_CPRE (1 << 1) #define ARM_MMU500_ACR_CACHE_LOCK (1 << 26) @@ -422,7 +427,7 @@ static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu, { unsigned int spin_cnt, delay; - writel_relaxed(0, sync); + qcom_writel(0, sync); for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) { for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) { if (!(readl_relaxed(status) & sTLBGSTATUS_GSACTIVE)) @@ -1760,8 +1765,8 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu) } /* Invalidate the TLB, just in case */ - writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH); - writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH); + qcom_writel(0, gr0_base + ARM_SMMU_GR0_TLBIALLH); + qcom_writel(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH); reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0); Can a quirk be used to work around the issue? Or can we just "pessimize" the 3 writes for everybody? (Might be cheaper than a test anyway) Regards. 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 X-Spam-Level: X-Spam-Status: No, score=-3.7 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4D4AAC43381 for ; Tue, 12 Mar 2019 12:37:02 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 1B46B2171F for ; Tue, 12 Mar 2019 12:37:02 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="UAR134cB" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1B46B2171F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=free.fr Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=1lSvn1KfygiS8XUl3Re75honJvx1/yUSVt5zXfUh0QQ=; b=UAR134cBenSDKh YUUUAK66bqRk9k3Fdr0ni/OCjx0VMbuEWfquvUAeZhZprEJ/Pk8/uB8j0ZO+uCHaoVWNmWOZg+78l halpNc8xRScZOL86nnFeEDrDAmIgPy/x81J6EM0QIvsBOxqD8mJ9FyWqezOXQtLuWA3igqsVY6Z17 OYYJevu3yN3vo0Gai0tUK0LAEt2oITAUQAtdgVXURbrkSXmw479FAMfmjCorT1l6dsh9nfhgcRQDr gjwOsovQoIay2eN8f95A8HNB9mUyEs/UiAZgf+p4zUNykJ1IzPsfIqjxhtfLqji6uMJYIHKlEEf7J a+t9jIeq67vRwRhhH+JQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h3geQ-0008Op-EO; Tue, 12 Mar 2019 12:36:58 +0000 Received: from smtp3-g21.free.fr ([2a01:e0c:1:1599::12]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h3geL-0008Nc-Uj for linux-arm-kernel@lists.infradead.org; Tue, 12 Mar 2019 12:36:56 +0000 Received: from [192.168.108.8] (unknown [213.36.7.13]) (Authenticated sender: marc.w.gonzalez) by smtp3-g21.free.fr (Postfix) with ESMTPSA id 24FC313F8B8; Tue, 12 Mar 2019 13:36:26 +0100 (CET) Subject: Re: [PATCH] arm64/io: Don't use WZR in writel To: Marc Zyngier , Will Deacon , Robin Murphy References: <68b71c15f32341468a868f6418e4fcb375bc49ba.camel@gmail.com> <20190211105755.GB30880@fuggles.cambridge.arm.com> <38d8965a-cd41-17cf-1b95-8dd58c079be4@arm.com> <874c702b8af760aa8fae38d478c79e3ecba00515.camel@gmail.com> <235d20ef-3054-69d9-975d-25aebf32aad3@arm.com> <20190223181254.GC572@tuxbook-pro> <86zhqm8i6d.wl-marc.zyngier@arm.com> <20190224035356.GD572@tuxbook-pro> From: Marc Gonzalez Message-ID: <33d765b5-1807-fa6c-1ceb-99f09f7c8d5a@free.fr> Date: Tue, 12 Mar 2019 13:36:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <20190224035356.GD572@tuxbook-pro> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190312_053654_291071_4B57A0BC X-CRM114-Status: GOOD ( 19.12 ) 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: Jens Axboe , Jeffrey Hugo , Catalin Marinas , LKML , Bjorn Andersson , MSM , AngeloGioacchino Del Regno , Linux ARM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 24/02/2019 04:53, Bjorn Andersson wrote: > On Sat 23 Feb 10:37 PST 2019, Marc Zyngier wrote: > >> On Sat, 23 Feb 2019 18:12:54 +0000, Bjorn Andersson wrote: >>> >>> On Mon 11 Feb 06:59 PST 2019, Marc Zyngier wrote: >>> >>>> On 11/02/2019 14:29, AngeloGioacchino Del Regno wrote: >>>> >>>>> Also, just one more thing: yes this thing is going ARM64-wide and >>>>> - from my findings - it's targeting certain Qualcomm SoCs, but... >>>>> I'm not sure that only QC is affected by that, others may as well >>>>> have the same stupid bug. >>>> >>>> At the moment, only QC SoCs seem to be affected, probably because >>>> everyone else has debugged their hypervisor (or most likely doesn't >>>> bother with shipping one). >>>> >>>> In all honesty, we need some information from QC here: which SoCs are >>>> affected, what is the exact nature of the bug, can it be triggered from >>>> EL0. Randomly papering over symptoms is not something I really like >>>> doing, and is likely to generate problems on unaffected systems. >>> >>> The bug at hand is that the XZR is not deemed a valid source in the >>> virtualization of the SMMU registers. It was identified and fixed for >>> all platforms that are shipping kernels based on v4.9 or later. >> >> When you say "fixed": Do you mean fixed in the firmware? Or by adding >> a workaround in the shipped kernel? > > I mean that it's fixed in the firmware. > >> If the former, is this part of an official QC statement, with an >> associated erratum number? > > I don't know, will get back to you on this one. > >> Is this really limited to the SMMU accesses? > > Yes. > >>> As such Angelo's list of affected platforms covers the high-profile >>> ones. In particular MSM8996 and MSM8998 is getting pretty good support >>> upstream, if we can figure out a way around this issue. >> >> We'd need an exhaustive list of the affected SoCs, and work out if we >> can limit the hack to the SMMU driver (cc'ing Robin, who's the one >> who'd know about it). > > I will try to compose a list. FWIW, I have just been bitten by this issue. I needed to enable an SMMU to filter PCIe EP accesses to system RAM (or something). I'm using an APQ8098 MEDIABOX dev board. My system hangs in arm_smmu_device_reset() doing: /* Invalidate the TLB, just in case */ writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH); writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH); With the 'Z' constraint, gcc generates: str wzr, [x0] without the 'Z' constraint, gcc generates: mov w1, 0 str w1, [x0] I can work around the problem using the following patch: diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 045d93884164..93117519aed8 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -59,6 +59,11 @@ #include "arm-smmu-regs.h" +static inline void qcom_writel(u32 val, volatile void __iomem *addr) +{ + asm volatile("str %w0, [%1]" : : "r" (val), "r" (addr)); +} + #define ARM_MMU500_ACTLR_CPRE (1 << 1) #define ARM_MMU500_ACR_CACHE_LOCK (1 << 26) @@ -422,7 +427,7 @@ static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu, { unsigned int spin_cnt, delay; - writel_relaxed(0, sync); + qcom_writel(0, sync); for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) { for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) { if (!(readl_relaxed(status) & sTLBGSTATUS_GSACTIVE)) @@ -1760,8 +1765,8 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu) } /* Invalidate the TLB, just in case */ - writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLH); - writel_relaxed(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH); + qcom_writel(0, gr0_base + ARM_SMMU_GR0_TLBIALLH); + qcom_writel(0, gr0_base + ARM_SMMU_GR0_TLBIALLNSNH); reg = readl_relaxed(ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0); Can a quirk be used to work around the issue? Or can we just "pessimize" the 3 writes for everybody? (Might be cheaper than a test anyway) Regards. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel