From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9824A6FA4 for ; Wed, 2 Aug 2023 10:52:09 +0000 (UTC) Received: from canpemm500006.china.huawei.com (unknown [172.30.72.54]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4RG7xh4fjKzNmZ3; Wed, 2 Aug 2023 18:48:40 +0800 (CST) Received: from [10.67.145.224] (10.67.145.224) by canpemm500006.china.huawei.com (7.192.105.130) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.27; Wed, 2 Aug 2023 18:52:06 +0800 Subject: Re: [PATCH 1/1] Revert "iommu/arm-smmu-v3: Set TTL invalidation hint better" To: Will Deacon CC: , , , Robin Murphy , Joerg Roedel , Lu Baolu , Jason Gunthorpe , Yicong Yang , Tomas Krcka , Jean-Philippe Brucker , Nicolin Chen References: <1690784482-30028-1-git-send-email-wangwudi@hisilicon.com> <20230801085504.GA26130@willie-the-truck> From: zhurui Message-ID: <27c895b8-1fb0-be88-8bc3-878d754684c8@huawei.com> Date: Wed, 2 Aug 2023 18:52:05 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 Precedence: bulk X-Mailing-List: iommu@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In-Reply-To: <20230801085504.GA26130@willie-the-truck> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.145.224] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To canpemm500006.china.huawei.com (7.192.105.130) X-CFilter-Loop: Reflected On 2023/8/1 16:55, Will Deacon wrote: > On Mon, Jul 31, 2023 at 02:21:22PM +0800, wangwudi wrote: >> From: Rui Zhu >> >> This reverts commit 6833b8f2e19945a41e4d5efd8c6d9f4cae9a5b7d. >> >> This constraint violates the protocol. When tg is not 0 but ttl, scale, >> and num are 0, the hardware reports the CERROR_IL gerror. In the >> protocol, leaf is not a prerequisite for TTL. >> >> Cc: Will Deacon >> Cc: Robin Murphy >> Cc: Joerg Roedel >> Cc: Lu Baolu >> Cc: Jason Gunthorpe >> Cc: Yicong Yang >> Cc: Tomas Krcka >> Cc: Jean-Philippe Brucker >> Cc: Nicolin Chen >> Cc: Rui Zhu >> >> Signed-off-by: Rui Zhu >> --- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 9 ++------- >> 1 file changed, 2 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> index 9b0dc3505601..098e84cfa82f 100644 >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> @@ -1898,13 +1898,8 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd, >> /* Convert page size of 12,14,16 (log2) to 1,2,3 */ >> cmd->tlbi.tg = (tg - 10) / 2; >> >> - /* >> - * Determine what level the granule is at. For non-leaf, io-pgtable >> - * assumes .tlb_flush_walk can invalidate multiple levels at once, >> - * so ignore the nominal last-level granule and leave TTL=0. >> - */ >> - if (cmd->tlbi.leaf) >> - cmd->tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3)); >> + /* Determine what level the granule is at */ >> + cmd->tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3)); > > Doesn't this reintroduce the bug that 6833b8f2e199 tried to fix? > > afaict, we should only hit the problematic case of tg != 0 but ttl, scale > and num all 0 if we're invalidating a single page, so shouldn't we just > zap tg in that case, since it's not doing anything useful? You're right. I'm sorry I missed. I just need to handle the problematic case by assigning 0 to tg. It's better to add this following code before each tlbi cmd batch add. if (num_pages == 1) { cmd->tlbi.tg = 0; } I'll resubmit a new patch. Thanks for your correction. > > I hesitate to say we should avoid range invalidation altogether for > single-page invalidations because I think some errata workarounds might > need that to work. > > Will > . >