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 C276DC4829E for ; Thu, 15 Feb 2024 17:27:34 +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=+djllBKevanbRwhIsa6RdtfZfA0qfCAJAaGX6v0kjK4=; b=r4Hcn5i/6yKZSd Ny4AWvIR0o5RPt1ZWaVcc897LC052x/zJ9ePMhqd78TJ4jHgIqR+HZnNSIvnBIc7XMCrlHh8y7m2d rarhP9t2Te3IoaGmNO+LSmHAxbpsbEisvFMEMS0RE7YMLM6O4cbZMSousYEiHW+qFA1eMsPotV2n5 MbZNkd4f2SjaqeKRactwkw4Ma7fiGK8HySrIS/8a1OcGFaVwbfaWSiy7xm5z+KBxkhNnE7FruwizG +n4RrEQQr8+JMKoA3wvrvrNQkVUj+BLYN7PfsN2VQU3QP7SNTfUnkBaFtfKh9EkOVl+F4dcdWeIqq Lp0Q0Nyn5VMRqkmxE/Xg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rafW3-0000000HAct-3CZj; Thu, 15 Feb 2024 17:27:19 +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 1rafW0-0000000HAap-1deV for linux-arm-kernel@lists.infradead.org; Thu, 15 Feb 2024 17:27:17 +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 877B51FB; Thu, 15 Feb 2024 09:27:54 -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 AB0A13F766; Thu, 15 Feb 2024 09:27:11 -0800 (PST) Message-ID: <2ca7f64e-a9e3-4d47-b04f-726c86223fd6@arm.com> Date: Thu, 15 Feb 2024 17:27:09 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 02/17] iommu/arm-smmu-v3: Consolidate the STE generation for abort/bypass Content-Language: en-GB To: Jason Gunthorpe , iommu@lists.linux.dev, Joerg Roedel , linux-arm-kernel@lists.infradead.org, Will Deacon Cc: Lu Baolu , Jean-Philippe Brucker , Joerg Roedel , Moritz Fischer , Moritz Fischer , Michael Shavit , Nicolin Chen , patches@lists.linux.dev, Shameer Kolothum , Mostafa Saleh , Zhangfei Gao References: <2-v5-cd1be8dd9c71+3fa-smmuv3_newapi_p1_jgg@nvidia.com> From: Robin Murphy In-Reply-To: <2-v5-cd1be8dd9c71+3fa-smmuv3_newapi_p1_jgg@nvidia.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240215_092716_567370_B8CC5ED4 X-CRM114-Status: GOOD ( 28.52 ) 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 06/02/2024 3:12 pm, Jason Gunthorpe wrote: > This allows writing the flow of arm_smmu_write_strtab_ent() around abort > and bypass domains more naturally. > > Note that the core code no longer supplies NULL domains, though there is > still a flow in the driver that end up in arm_smmu_write_strtab_ent() with > NULL. A later patch will remove it. > > Remove the duplicate calculation of the STE in arm_smmu_init_bypass_stes() > and remove the force parameter. arm_smmu_rmr_install_bypass_ste() can now > simply invoke arm_smmu_make_bypass_ste() directly. > > Reviewed-by: Michael Shavit > Reviewed-by: Nicolin Chen > Reviewed-by: Mostafa Saleh > Tested-by: Shameer Kolothum > Tested-by: Nicolin Chen > Tested-by: Moritz Fischer > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 97 ++++++++++++--------- > 1 file changed, 55 insertions(+), 42 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 f0b915567cbcdc..6123e5ad95822c 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -1498,6 +1498,24 @@ static void arm_smmu_write_ste(struct arm_smmu_master *master, u32 sid, > } > } > > +static void arm_smmu_make_abort_ste(struct arm_smmu_ste *target) > +{ > + memset(target, 0, sizeof(*target)); > + target->data[0] = cpu_to_le64( > + STRTAB_STE_0_V | > + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT)); > +} > + > +static void arm_smmu_make_bypass_ste(struct arm_smmu_ste *target) > +{ > + memset(target, 0, sizeof(*target)); > + target->data[0] = cpu_to_le64( > + STRTAB_STE_0_V | > + FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS)); > + target->data[1] = cpu_to_le64( > + FIELD_PREP(STRTAB_STE_1_SHCFG, STRTAB_STE_1_SHCFG_INCOMING)); > +} > + > static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid, > struct arm_smmu_ste *dst) > { > @@ -1508,37 +1526,31 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid, > struct arm_smmu_domain *smmu_domain = master->domain; > struct arm_smmu_ste target = {}; > > - if (smmu_domain) { > - switch (smmu_domain->stage) { > - case ARM_SMMU_DOMAIN_S1: > - cd_table = &master->cd_table; > - break; > - case ARM_SMMU_DOMAIN_S2: > - s2_cfg = &smmu_domain->s2_cfg; > - break; > - default: > - break; > - } > + if (!smmu_domain) { > + if (disable_bypass) > + arm_smmu_make_abort_ste(&target); > + else > + arm_smmu_make_bypass_ste(&target); > + arm_smmu_write_ste(master, sid, dst, &target); > + return; > + } > + > + switch (smmu_domain->stage) { > + case ARM_SMMU_DOMAIN_S1: > + cd_table = &master->cd_table; > + break; > + case ARM_SMMU_DOMAIN_S2: > + s2_cfg = &smmu_domain->s2_cfg; > + break; > + case ARM_SMMU_DOMAIN_BYPASS: > + arm_smmu_make_bypass_ste(&target); > + arm_smmu_write_ste(master, sid, dst, &target); > + return; > } > > /* Nuke the existing STE_0 value, as we're going to rewrite it */ > val = STRTAB_STE_0_V; > > - /* Bypass/fault */ > - if (!smmu_domain || !(cd_table || s2_cfg)) { > - if (!smmu_domain && disable_bypass) > - val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT); > - else > - val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS); > - > - target.data[0] = cpu_to_le64(val); > - target.data[1] = cpu_to_le64(FIELD_PREP(STRTAB_STE_1_SHCFG, > - STRTAB_STE_1_SHCFG_INCOMING)); > - target.data[2] = 0; /* Nuke the VMID */ > - arm_smmu_write_ste(master, sid, dst, &target); > - return; > - } > - > if (cd_table) { > u64 strw = smmu->features & ARM_SMMU_FEAT_E2H ? > STRTAB_STE_1_STRW_EL2 : STRTAB_STE_1_STRW_NSEL1; > @@ -1583,22 +1595,20 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid, > arm_smmu_write_ste(master, sid, dst, &target); > } > > +/* > + * This can safely directly manipulate the STE memory without a sync sequence > + * because the STE table has not been installed in the SMMU yet. > + */ > static void arm_smmu_init_bypass_stes(struct arm_smmu_ste *strtab, This name is long out-of-date - if we're refreshing this area, please rename to something relevant to what it actually does, e.g. s/bypass/initial/. Although frankly I also think that at this point we should just get rid of the disable_bypass parameter altogether - it's been almost entirely meaningless since default domain support was added, and any tenuous cases for wanting inital STEs to be bypass should probably be using RMRs now anyway. Thanks, Robin. > - unsigned int nent, bool force) > + unsigned int nent) > { > unsigned int i; > - u64 val = STRTAB_STE_0_V; > - > - if (disable_bypass && !force) > - val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT); > - else > - val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS); > > for (i = 0; i < nent; ++i) { > - strtab->data[0] = cpu_to_le64(val); > - strtab->data[1] = cpu_to_le64(FIELD_PREP( > - STRTAB_STE_1_SHCFG, STRTAB_STE_1_SHCFG_INCOMING)); > - strtab->data[2] = 0; > + if (disable_bypass) > + arm_smmu_make_abort_ste(strtab); > + else > + arm_smmu_make_bypass_ste(strtab); > strtab++; > } > } > @@ -1626,7 +1636,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid) > return -ENOMEM; > } > > - arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT, false); > + arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT); > arm_smmu_write_strtab_l1_desc(strtab, desc); > return 0; > } > @@ -3245,7 +3255,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu) > reg |= FIELD_PREP(STRTAB_BASE_CFG_LOG2SIZE, smmu->sid_bits); > cfg->strtab_base_cfg = reg; > > - arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents, false); > + arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents); > return 0; > } > > @@ -3956,7 +3966,6 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu) > iort_get_rmr_sids(dev_fwnode(smmu->dev), &rmr_list); > > list_for_each_entry(e, &rmr_list, list) { > - struct arm_smmu_ste *step; > struct iommu_iort_rmr_data *rmr; > int ret, i; > > @@ -3969,8 +3978,12 @@ static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu) > continue; > } > > - step = arm_smmu_get_step_for_sid(smmu, rmr->sids[i]); > - arm_smmu_init_bypass_stes(step, 1, true); > + /* > + * STE table is not programmed to HW, see > + * arm_smmu_init_bypass_stes() > + */ > + arm_smmu_make_bypass_ste( > + arm_smmu_get_step_for_sid(smmu, rmr->sids[i])); > } > } > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel