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=-9.8 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS 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 089E3C433E4 for ; Sun, 26 Jul 2020 17:04:00 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 BB1FF2070B for ; Sun, 26 Jul 2020 17:03:59 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="oBa0Z4AW"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="FNWC0Xgy" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BB1FF2070B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=HPyAnMOJwEMCuVV/Ha0Nkeq4U3f+lM4CbGS8EeFfX68=; b=oBa0Z4AWjqfy9bcR6tMNWiUWl ZxBHreDGpBkKO16HtHGuYHv1d3U3+drf/lIOdsIGTGuj/skdANPKxWZah3jUyWt7iPHlBEeSvlVek QASTVwy7mmOFqk5R25iuFHPcEy8ovOUubSDcSigX5yA+SmxPDb01ZvyzzWw/Br3IOnYuI57bvW6K8 XxTLtDBt0f+rLIbZSslPLrS2etNhi5tD/qPFuLB6bPi06alp6mvtPCDUaJ7W5Z1KNI66V/jDCvpI9 TJx+0oCLHApg7zfCobCC4UpquLoQlaVF1H/BI5y6Wco3YiS0PrUI1MsGI14JhGP86u2CizJEMccvK c9sue8YRQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jzk2j-0001le-Fw; Sun, 26 Jul 2020 17:02:33 +0000 Received: from mail-ed1-x544.google.com ([2a00:1450:4864:20::544]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jzk2g-0001lB-Nt for linux-arm-kernel@lists.infradead.org; Sun, 26 Jul 2020 17:02:31 +0000 Received: by mail-ed1-x544.google.com with SMTP id c15so780733edj.3 for ; Sun, 26 Jul 2020 10:02:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=vqJigkc3igUS2cVEm8BI8huTNBWytoNhNONUxIK8JdE=; b=FNWC0XgyURknaBPIMonO4L8XBAugVby97t8RO9h6h82Sf7+Tl0KnbZ52IJjMRlzKwd scOI1cXajPHYazMGJh5bZKd/QtdBFV2dQfFHCqZCpZWPbLvB5nmIFLhtdrdpr5gR78TI XdJTfUwUVjqGAMOS5SuPNFbngNF9jax6Re2Dcp7Aerv/Thw5IJ1dq8wZrdJ74yQ2n/dW 1wgJmZ8o4oU17Ya4KmPGtJFYH7MGWcOFU4Of5q9PfHi8eoe7EpxS4cP5AWTwVecirrcI gP9URO0n06KykrfFdCC9CdsYicPETYQQddu2KA7Zs8D4ooPX2kANacUCMprGpfqV3QbQ 23Lw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=vqJigkc3igUS2cVEm8BI8huTNBWytoNhNONUxIK8JdE=; b=kMFBTSNUr6jgk75CYImB5TI8TFPMHm0WQbqp5pndoS+2mYthqsVhU2jbiuCR1zmIWM cN1+xWQtxmByUYqgxC962jd7jYiB12Qqm4N607WxW2SFipryZuFXAelgZVX+1vW5do7z BL/B1nokhBQ8fr+bSpMdJwNIT6evMpejpzrOLb/DssVZb0KUgFDNT39X8ZYzXqb/dAd1 GWOgqiIhmdkL0plTsRCENokOSqPKvnRxHIb8APrFQce1OxK+q3Ms+gkGKCJYW2FXXvYd /TZVFPf76fYlNc17GPz2LItHLcBowrKud4BbTWwvRqbv6V3sd45quARWlzT5rM1E/bUq zKzQ== X-Gm-Message-State: AOAM532XuKzjcScVvrXZQD/V63fKHIlb2LxHf+kxZfJ25grm3gmn/GGP 5gBnmR15sNMfsEkHvd5aLRupCE94rBo4DsqAa+o= X-Google-Smtp-Source: ABdhPJz82dpeEPbR1Xtfmkl2JtXHTjStwPUWLpKbFISs6ED3u33doh8XAyCMytHvaqDpkWq/Wga9lMPNWZaDSUGrtEI= X-Received: by 2002:a05:6402:a5b:: with SMTP id bt27mr1535161edb.120.1595782948944; Sun, 26 Jul 2020 10:02:28 -0700 (PDT) MIME-Version: 1.0 References: <20200720154047.3611092-1-jcrouse@codeaurora.org> <20200720154047.3611092-7-jcrouse@codeaurora.org> In-Reply-To: <20200720154047.3611092-7-jcrouse@codeaurora.org> From: Rob Clark Date: Sun, 26 Jul 2020 10:03:07 -0700 Message-ID: Subject: Re: [Freedreno] [PATCH v10 06/13] iommu/arm-smmu-qcom: Get and set the pagetable config for split pagetables To: Jordan Crouse X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200726_130230_816291_9A0B0DC6 X-CRM114-Status: GOOD ( 33.43 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: freedreno , Sai Prakash Ranjan , Will Deacon , linux-arm-msm , Joerg Roedel , Linux Kernel Mailing List , Bjorn Andersson , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Robin Murphy , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Jul 20, 2020 at 8:41 AM Jordan Crouse wrote: > > The Adreno GPU has the capability to manage its own pagetables and switch > them dynamically from the hardware. To do this the GPU uses TTBR1 for > "global" GPU memory and creates local pagetables for each context and > switches them dynamically with the GPU. > > Use DOMAIN_ATTR_PGTABLE_CFG to get the current configuration for the > TTBR1 pagetable from the smmu driver so the leaf driver can create > compatible pagetables for use with TTBR0. > > Because TTBR0 is disabled by default when TTBR1 is enabled the GPU > driver can pass the configuration of one of the newly created pagetables > back through DOMAIN_ATTR_PGTABLE_CFG as a trigger to enable translation on > TTBR0. > > Signed-off-by: Jordan Crouse > --- > > drivers/iommu/arm-smmu-qcom.c | 47 +++++++++++++++++++++++++++++++++++ > drivers/iommu/arm-smmu.c | 32 ++++++++++++++++++------ > drivers/iommu/arm-smmu.h | 10 ++++++++ > 3 files changed, 81 insertions(+), 8 deletions(-) > > diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c > index b9a5c5369e86..9a0c64ca9cb6 100644 > --- a/drivers/iommu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm-smmu-qcom.c > @@ -34,6 +34,52 @@ static bool qcom_adreno_smmu_is_gpu_device(struct device *dev) > return false; > } > > +/* > + * Local implementation to configure TTBR0 wil the specified pagetable config. > + * The GPU driver will call this to enable TTBR0 when per-instance pagetables > + * are active > + */ > +static int qcom_adreno_smmu_set_pgtable_cfg(struct arm_smmu_domain *smmu_domain, > + struct io_pgtable_cfg *pgtbl_cfg) > +{ > + struct io_pgtable *pgtable = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops); > + struct arm_smmu_cfg *cfg = &smmu_domain->cfg; > + struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx]; > + > + /* The domain must have split pagetables already enabled */ > + if (cb->tcr[0] & ARM_SMMU_TCR_EPD1) > + return -EINVAL; > + > + /* If the pagetable config is NULL, disable TTBR0 */ > + if (!pgtbl_cfg) { > + /* Do nothing if it is already disabled */ > + if ((cb->tcr[0] & ARM_SMMU_TCR_EPD0)) > + return -EINVAL; > + > + /* Set TCR to the original configuration */ > + cb->tcr[0] = arm_smmu_lpae_tcr(&pgtable->cfg); > + cb->ttbr[0] = FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid); > + } else { > + u32 tcr = cb->tcr[0]; > + > + /* FIXME: What sort of validation do we need to do here? */ > + > + /* Don't call this again if TTBR0 is already enabled */ > + if (!(cb->tcr[0] & ARM_SMMU_TCR_EPD0)) > + return -EINVAL; > + > + tcr |= arm_smmu_lpae_tcr(pgtbl_cfg); > + tcr &= ~(ARM_SMMU_TCR_EPD0 | ARM_SMMU_TCR_EPD1); > + > + cb->tcr[0] = tcr; > + cb->ttbr[0] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr; > + cb->ttbr[0] |= FIELD_PREP(ARM_SMMU_TTBRn_ASID, cb->cfg->asid); > + } > + > + arm_smmu_write_context_bank(smmu_domain->smmu, cb->cfg->cbndx); > + return 0; > +} > + > static int qcom_adreno_smmu_alloc_context_bank(struct arm_smmu_domain *smmu_domain, > struct device *dev, int start, int count) > { > @@ -131,6 +177,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_impl = { > .def_domain_type = qcom_smmu_def_domain_type, > .reset = qcom_smmu500_reset, > .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, > + .set_pgtable_cfg = qcom_adreno_smmu_set_pgtable_cfg, > }; > > static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu, > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index fff536a44faa..e1036ae54a8d 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -86,13 +86,6 @@ struct arm_smmu_smr { > bool valid; > }; > > -struct arm_smmu_cb { > - u64 ttbr[2]; > - u32 tcr[2]; > - u32 mair[2]; > - struct arm_smmu_cfg *cfg; > -}; > - > static bool using_legacy_binding, using_generic_binding; > > static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu) > @@ -558,7 +551,7 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain, > } > } > > -static void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx) > +void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx) > { > u32 reg; > bool stage1; > @@ -1515,6 +1508,18 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain, > case DOMAIN_ATTR_NESTING: > *(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED); > return 0; > + case DOMAIN_ATTR_PGTABLE_CFG: { > + struct io_pgtable *pgtable; > + struct io_pgtable_cfg *dest = data; > + > + if (!smmu_domain->pgtbl_ops) > + return -ENODEV; > + > + pgtable = io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops); > + > + memcpy(dest, &pgtable->cfg, sizeof(*dest)); > + return 0; > + } hmm, maybe it would make sense to have impl hooks for get/set_attr, so we could handle DOMAIN_ATTR_PGTABLE_CFG inside the adreno_smmu_impl? Having impl specific domain attrs would be useful for what I have in mind to enable stall/resume support, so we can hook in devcoredump to iova faults (which would be a huge improvement for debugability, right now iova faults are somewhat harder to debug than needed). My rough idea was to add DOMAIN_ATTR_RESUME, which could be used with set_attr() to (1) enable STALL and let drm/msm know whether the iommu supports it, and (2) resume translation from wq context after devcoredump snapshot is collected. BR, -R > default: > return -ENODEV; > } > @@ -1555,6 +1560,17 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain, > else > smmu_domain->stage = ARM_SMMU_DOMAIN_S1; > break; > + case DOMAIN_ATTR_PGTABLE_CFG: { > + struct arm_smmu_device *smmu = smmu_domain->smmu; > + > + ret = -EPERM; > + > + if (smmu) > + if (smmu->impl && smmu->impl->set_pgtable_cfg) > + ret = smmu->impl->set_pgtable_cfg(smmu_domain, > + data); > + } > + break; > default: > ret = -ENODEV; > } > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h > index 9f81c1fffe1e..9325fc28d24a 100644 > --- a/drivers/iommu/arm-smmu.h > +++ b/drivers/iommu/arm-smmu.h > @@ -328,6 +328,13 @@ struct arm_smmu_cfg { > }; > #define ARM_SMMU_INVALID_IRPTNDX 0xff > > +struct arm_smmu_cb { > + u64 ttbr[2]; > + u32 tcr[2]; > + u32 mair[2]; > + struct arm_smmu_cfg *cfg; > +}; > + > enum arm_smmu_domain_stage { > ARM_SMMU_DOMAIN_S1 = 0, > ARM_SMMU_DOMAIN_S2, > @@ -408,6 +415,8 @@ struct arm_smmu_impl { > int (*def_domain_type)(struct device *dev); > int (*alloc_context_bank)(struct arm_smmu_domain *smmu_domain, > struct device *dev, int start, int max); > + int (*set_pgtable_cfg)(struct arm_smmu_domain *smmu_domain, > + struct io_pgtable_cfg *cfg); > }; > > #define INVALID_SMENDX -1 > @@ -493,6 +502,7 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu); > struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu); > struct arm_smmu_device *qcom_adreno_smmu_impl_init(struct arm_smmu_device *smmu); > > +void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx); > int arm_mmu500_reset(struct arm_smmu_device *smmu); > > #endif /* _ARM_SMMU_H */ > -- > 2.25.1 > > _______________________________________________ > Freedreno mailing list > Freedreno@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel