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 2FECFC6FD19 for ; Mon, 13 Mar 2023 13:08:58 +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=Meeqzhr4BtN/hLcYXCTGIMzU475U7bvrXY5J5aiNlHI=; b=x3ubA93oQ4ipP9 Sf38w6k//un5DZXlvQM5YEfPwkdWrdsu9wbNSkeM+U93YRPmh2MdHast7cfL/aZzH18sDiA/xN1Lw JibgqDmUOXvFlyPnDmLMdrch5/g9xYuwaUD62dEukqnaoVbo2Q3SNEursn4L9cYQdme+H3QkmCfxh TUsO/tLOdW37OFh1QYjpTleFAaZn/sx5q32ZjzbqOThKpgdhDGCUz0NvdN6mBr95zs6z9cbcZdj6K c74hnBAG2at+Ob4WiKiyNYx631IzycQwUMrv6xVVkX7uoKX+04sxiNNY8kNv5/RnkWE5MtIqhth+E NEz/NuE2CCFUBZjNKaUw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pbhuA-005mqS-8C; Mon, 13 Mar 2023 13:07:58 +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 1pbhu5-005mp9-3y for linux-arm-kernel@lists.infradead.org; Mon, 13 Mar 2023 13:07:55 +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 64AD91FB; Mon, 13 Mar 2023 06:08:31 -0700 (PDT) Received: from [10.57.91.139] (unknown [10.57.91.139]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BB5C43F64C; Mon, 13 Mar 2023 06:07:45 -0700 (PDT) Message-ID: Date: Mon, 13 Mar 2023 13:07:42 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH v1 14/14] iommu/arm-smmu-v3: Add arm_smmu_cache_invalidate_user Content-Language: en-GB To: Nicolin Chen Cc: jgg@nvidia.com, will@kernel.org, eric.auger@redhat.com, kevin.tian@intel.com, baolu.lu@linux.intel.com, joro@8bytes.org, shameerali.kolothum.thodi@huawei.com, jean-philippe@linaro.org, linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org References: <1467e666-1b6c-c285-3f79-f8e8b088718b@arm.com> <92fdb06f-e5b1-8534-fb0e-ad47b5be9e1d@arm.com> From: Robin Murphy In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230313_060753_299915_D4A01C0C X-CRM114-Status: GOOD ( 21.55 ) 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 2023-03-11 12:38, Nicolin Chen wrote: > On Fri, Mar 10, 2023 at 05:53:46PM +0000, Robin Murphy wrote: > >>>>> + case CMDQ_OP_TLBI_NH_VA: >>>>> + cmd.tlbi.asid = inv_info->asid; >>>>> + fallthrough; >>>>> + case CMDQ_OP_TLBI_NH_VAA: >>>>> + if (!granule_size || !(granule_size & smmu->pgsize_bitmap) || >>>> >>>> Non-range invalidations with TG=0 are perfectly legal, and should not be >>>> ignored. >>> >>> I assume that you are talking about the pgsize_bitmap check. >>> >>> QEMU embeds a !tg case into the granule_size [1]. So it might >>> not be straightforward to cover that case. Let me see how to >>> untangle different cases and handle them accordingly. >> >> Oh, double-checking patch #2, that might be me misunderstanding the >> interface. I hadn't realised that the UAPI was apparently modelled on >> arm_smmu_tlb_inv_range_asid() rather than actual SMMU commands :) > > Yea. In fact, most of the invalidation info in QEMU was packed > for the previously defined general cache invalidation structure, > and the range invalidation part is still not quite independent. > >> I really think UAPI should reflect the hardware and encode TG and TTL >> directly. Especially since there's technically a flaw in the current >> driver where we assume TTL in cases where it isn't actually known, thus >> may potentially fail to invalidate level 2 block entries when removing a >> level 1 table, since io-pgtable passes the level 3 granule in that case. > > Do you mean something like hw_info forwarding pgsize_bitmap/tg > to the guest? Or the other direction? I mean if the interface wants to support range invalidations in a way which works correctly, then it should ideally carry both the TG and TTL fields from the guest command straight through to the host. If not, then at the very least the host must always assume TTL=0, because it cannot correctly infer otherwise once the guest command's original intent has been lost. >> When range invalidation came along, the distinction between "all leaves >> are definitely at the last level" and "use last-level granularity to >> make sure everything at at any level is hit" started to matter, but the >> interface never caught up. It hasn't seemed desperately urgent to fix >> (who does 1GB+ unmaps outside of VFIO teardown anyway?), but we must >> definitely not bake the same mistake into user ABI. >> >> Of course, there might then be cases where we need to transform >> non-range commands into range commands for the sake of workarounds, but >> that's our own problem to deal with. > > Noted it down. > >>>> What about NSNH_ALL? That still needs to invalidate all the S1 context >>>> that the guest *thinks* it's invalidating. >>> >>> NSNH_ALL is translated to NH_ALL at the guest level. But maybe >>> it should have been done here instead. >> >> Yes. It seems the worst of both worlds to have an interface which takes >> raw opcodes rather than an enum of supported commands, but still >> requires userspace to know which opcodes are supported and which ones >> don't work as expected even though they are entirely reasonable to use >> in the context of the stage-1-only SMMU being emulated. > > Maybe a list of supported TLBI commands via the hw_info uAPI? I don't think it's all that difficult to implicitly support all commands that are valid for a stage-1-only SMMU, it just needs the right interface design to be capable of encoding them all completely and unambiguously. Coming back to the previous point about the address encoding, I think that means basing it more directly on the actual SMMUv3 commands, rather than on io-pgtable's abstraction of invalidation with SMMUv3 opcodes bolted on. Thanks, Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel