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 36875C52D7C for ; Fri, 9 Aug 2024 18:35:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type: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=jkXZrKsMy0CK9Sl2C/Wzml5giLiIZuw/6AOqGwMtZ5I=; b=yYlaJZaQFs+4EkI7abqHen/JSj WJydvV+1LZAWxxdD+/grfVazWVhUsI64v4lRo/AenMAvIh6U2eAeo1OsX3VaXF/Z58V0i9zxJoWLp A9l69euvGP7DlkvF89rGx7CFc00b6JMfmqUKp4zEhklFLXmg5mdznZDdOGSa9sssKtjzGKpfCptx5 A9oh13l7lCRajbmt0q6jd0ydQTPARHLpLR5QbUamEY3XI40mc5zLWCjsk+NASZOoV6otfkMUgYMRE omenWBCjgXB05Wxlia4q3+j2M+orNACkFdJnEOAFRPFcAqZmSj5Ie+xfLOgTS2aDWaofY+d+ojpz1 mWhcx+SQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1scUS7-0000000CDUQ-0uEX; Fri, 09 Aug 2024 18:35:03 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1scURW-0000000CDPL-49OR for linux-arm-kernel@lists.infradead.org; Fri, 09 Aug 2024 18:34:28 +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 35B5013D5; Fri, 9 Aug 2024 11:34:51 -0700 (PDT) Received: from [10.57.46.232] (unknown [10.57.46.232]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DB8D03F6A8; Fri, 9 Aug 2024 11:34:21 -0700 (PDT) Message-ID: Date: Fri, 9 Aug 2024 19:34:20 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 6/8] iommu/arm-smmu-v3: Implement IOMMU_HWPT_ALLOC_NEST_PARENT To: Jason Gunthorpe Cc: acpica-devel@lists.linux.dev, Alex Williamson , Hanjun Guo , iommu@lists.linux.dev, Joerg Roedel , Kevin Tian , kvm@vger.kernel.org, Len Brown , linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Lorenzo Pieralisi , "Rafael J. Wysocki" , Robert Moore , Sudeep Holla , Will Deacon , Eric Auger , Jean-Philippe Brucker , Moritz Fischer , Michael Shavit , Nicolin Chen , patches@lists.linux.dev, Shameerali Kolothum Thodi References: <6-v1-54e734311a7f+14f72-smmuv3_nesting_jgg@nvidia.com> <20240809160959.GJ8378@nvidia.com> From: Robin Murphy Content-Language: en-GB In-Reply-To: <20240809160959.GJ8378@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240809_113427_151483_76E49E17 X-CRM114-Status: GOOD ( 26.67 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2024-08-09 5:09 pm, Jason Gunthorpe wrote: > On Fri, Aug 09, 2024 at 04:06:22PM +0100, Robin Murphy wrote: >> On 2024-08-07 12:41 am, Jason Gunthorpe wrote: >>> For SMMUv3 the parent must be a S2 domain, which can be composed >>> into a IOMMU_DOMAIN_NESTED. >>> >>> In future the S2 parent will also need a VMID linked to the VIOMMU and >>> even to KVM. >>> >>> Signed-off-by: Jason Gunthorpe >>> --- >>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> 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 6bbe4aa7b9511c..5faaccef707ef1 100644 >>> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >>> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >>> @@ -3103,7 +3103,8 @@ arm_smmu_domain_alloc_user(struct device *dev, u32 flags, >>> const struct iommu_user_data *user_data) >>> { >>> struct arm_smmu_master *master = dev_iommu_priv_get(dev); >>> - const u32 PAGING_FLAGS = IOMMU_HWPT_ALLOC_DIRTY_TRACKING; >>> + const u32 PAGING_FLAGS = IOMMU_HWPT_ALLOC_DIRTY_TRACKING | >>> + IOMMU_HWPT_ALLOC_NEST_PARENT; >>> struct arm_smmu_domain *smmu_domain; >>> int ret; >>> @@ -3116,6 +3117,14 @@ arm_smmu_domain_alloc_user(struct device *dev, u32 flags, >>> if (!smmu_domain) >>> return ERR_PTR(-ENOMEM); >>> + if (flags & IOMMU_HWPT_ALLOC_NEST_PARENT) { >>> + if (!(master->smmu->features & ARM_SMMU_FEAT_TRANS_S2)) { >> >> Nope, nesting needs to rely on FEAT_NESTING, that's why it exists. S2 alone >> isn't sufficient - without S1 there's nothing to expose to userspace, so >> zero point in having a "nested" domain with nothing to nest into it - but >> furthermore we need S2 *without* unsafe broken TLBs. > > I do tend to agree we should fail earlier if IOMMU_DOMAIN_NESTED is > not possible so let's narrow it. > > However, the above was matching how the driver already worked (ie the > old arm_smmu_enable_nesting()) where just asking for a normal S2 was > gated only by FEAT_S2. Ohhhh, I see, so actually the same old subtlety is still there - ALLOC_NEST_PARENT isn't a definite "allocate the parent domain for my nested setup", it's "allocate a domain which will be capable of being upgraded to nesting later *if* I choose to do so". Is the intent that someone could still use this if they had no intention of nesting but just wanted to ensure S2 format for their single stage of translation for some reason? It remains somewhat confusing since S2 domains on S2-only SMMUs are still fundamentally incapable of ever becoming a nested parent, but admittedly I'm struggling to think of a name which would be more accurate while still generic, so maybe it's OK... > This does add a CMDQ_OP_TLBI_NH_ALL, but I didn't think that hit an > errata? Indeed, all the really nasty errata depend on both stages being active (such that S1 translation requests interact with concurrent S2 invalidations) Thanks, Robin. > The nesting specific stuff that touches things that FEAT_NESTING > covers in the driver is checked here: > > static struct iommu_domain * > arm_smmu_domain_alloc_nesting(struct device *dev, u32 flags, > struct iommu_domain *parent, > const struct iommu_user_data *user_data) > { > if (!(master->smmu->features & ARM_SMMU_FEAT_NESTING)) > return ERR_PTR(-EOPNOTSUPP); > > Which prevents creating a IOMMU_DOMAIN_NESTED, meaning you can't get a > CD table on top of the S2 or issue any S1 invalidations. > > Thanks, > Jason