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 6031FC25B78 for ; Tue, 4 Jun 2024 16:07:51 +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-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=jjq9myayq4azAqCvAaQovcyuVtjMp8p+FdZsjhyZRWw=; b=38AZJ827Ax6RlC lQSY14oxWYY7gwDMHv6r7Rbj2FO1jHKLs5Hh+3/XuUuqB0L/jiS+e2KFM8wad19uvLjo+dsoYaaVE JNIqUPDzbM0sn8CNakLr9sK1xS3EONMYNZj0ckO9icsRay93cXSnp9ahO0qdTFV0j6IKPpJc/2OVE xmPzxiLvKEFtj/nwWUvoGCHHS9BuHqIqx85EsE3Iv/xT37D7jhoFriwPSp7BIDILhnEUghj2cQ/w0 jXU+TQK2VZxGt5GucNRaygvuDLb9o6DICOYzqrd4KbP3xO0YfOoaOiBdEocdRBOeoJd4ziwxzC8C3 KaVt1mqo3MQCr8ogXbxw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sEWhF-000000034Az-2Vxt; Tue, 04 Jun 2024 16:07:37 +0000 Received: from mail-wm1-x32c.google.com ([2a00:1450:4864:20::32c]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sEWhB-000000034A1-2wLp for linux-arm-kernel@lists.infradead.org; Tue, 04 Jun 2024 16:07:35 +0000 Received: by mail-wm1-x32c.google.com with SMTP id 5b1f17b1804b1-42152bb7b81so77225e9.0 for ; Tue, 04 Jun 2024 09:07:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1717517251; x=1718122051; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=j6IBMlbCwbxtgXzmlYBvTO7Gmj2Kf/XCsg/PolHHAtc=; b=QpRNwgyNtvVjf5tp+Uf0baLA3w6v3QUPywtqn9qoXE36iMDGUvvvGwmEInrA5BR54Y HhnzmdezlfjyrcabMRR01u0ru+pw2MtTQH4lAINwzRhxp/hOn5/eJaTWqv3cAjBGJrjV GayYdvMJtaNTbWDIsP90fn3IVS4zIdYRuSRnLWJNjDqyF0IylrPVxfr1KtBZLXj/j1YF xLx4ZxoYW06cjifB+63JagLYDhkpTz8M9V1ZFbrrL0t0dBtcJIF/uEpHAeNg06CHCXUN FhDlNa3d1AvUnYGJ54i9L/IdCP0e2nWMY0e8hZARrFeVfegIMll56kz3I+dtycdTCApO k9aw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717517251; x=1718122051; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=j6IBMlbCwbxtgXzmlYBvTO7Gmj2Kf/XCsg/PolHHAtc=; b=iNkZArDD2PYHlTSQcYc87HGzVVXnrdL/0wYsFoxzSoLzfkvgoQub8q2x/YacSSYokv tEJWw5fKmK+8eSwXUVn+zDL/2LdsUk6Dmq2cgbdoq7LUN5SNEG3LapXG448Fc3X3kozS QdJbSiEAfo4ON267C1yf7CXXjuqmAReToIpWBi6hGKK1ARplPQATrVESFGAP8MD8UHN9 euppN1J6/fh4TbiyFCnroOtWSnGhw5yplePEX2Edav0Zr+cI0IhEOm82QQJwhChFA//1 woj2rrVBvtpmdQK5IRx0YT0aB7bCYT5NTMhWlHhd4cFT7vfilVukqU6IKWLGWOImgsd+ KM+g== X-Forwarded-Encrypted: i=1; AJvYcCVGyUQRJMe72oC0vKiIdnhduxloCn0huIE/4g1zs3HSbpi7DwDDzzXgUM92OcPB+DRVoVqLMczYrsHzhJah6lO/5TJF9PByJZ+P7tT5g3sYAZGS10E= X-Gm-Message-State: AOJu0YzqEkZr40O8veWKOX0LyZIHxsfmScutPYpeeh4xp1teS4LvTlbM ygALyxVF6Cdo87jnENuA4HRadzRS1zj3eCWbPaBf+BuwOkQ2XdXqAoHDqDauAg== X-Google-Smtp-Source: AGHT+IGSdGh6/lK/tR/YkD9ol9SDg56kySSbj8FeCuBClEo8QYaxGvXB6ZFkC6gtdkLBtZglpOa7yQ== X-Received: by 2002:a05:600c:214e:b0:41a:444b:e1d9 with SMTP id 5b1f17b1804b1-4214947b4f8mr2953025e9.4.1717517245740; Tue, 04 Jun 2024 09:07:25 -0700 (PDT) Received: from google.com (230.213.79.34.bc.googleusercontent.com. [34.79.213.230]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-421381c650csm117293755e9.27.2024.06.04.09.07.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 04 Jun 2024 09:07:25 -0700 (PDT) Date: Tue, 4 Jun 2024 16:07:21 +0000 From: Mostafa Saleh To: Jason Gunthorpe Cc: iommu@lists.linux.dev, Joerg Roedel , linux-arm-kernel@lists.infradead.org, Robin Murphy , Will Deacon , Michael Shavit , Nicolin Chen , patches@lists.linux.dev, Ryan Roberts Subject: Re: [PATCH 4/7] iommu/arm-smmu-v3: Split struct arm_smmu_ctx_desc_cfg.cdtab Message-ID: References: <0-v1-1b720dce51d1+4f44-smmuv3_tidy_jgg@nvidia.com> <4-v1-1b720dce51d1+4f44-smmuv3_tidy_jgg@nvidia.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4-v1-1b720dce51d1+4f44-smmuv3_tidy_jgg@nvidia.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240604_090733_966927_496F2F71 X-CRM114-Status: GOOD ( 30.35 ) 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-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 Hi Jason, On Mon, Jun 03, 2024 at 07:31:30PM -0300, Jason Gunthorpe wrote: > This is being used as both an array of CDs and an array of L1 descriptors. > > Give the two usages different names and correct types. > > Remove CTXDESC_CD_DWORDS as most usages were indexing or sizing an array > of struct arm_smmu_cd. > > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 60 ++++++++++----------- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 17 ++++-- > 2 files changed, 41 insertions(+), 36 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 735dd9ff61890e..24c286a0834445 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c > @@ -1170,7 +1170,7 @@ static void arm_smmu_sync_cd(struct arm_smmu_master *master, > static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu, > struct arm_smmu_l1_ctx_desc *l1_desc) > { > - size_t size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3); > + size_t size = CTXDESC_L2_ENTRIES * sizeof(struct arm_smmu_cd); > > l1_desc->l2ptr = dmam_alloc_coherent(smmu->dev, size, > &l1_desc->l2ptr_dma, GFP_KERNEL); > @@ -1198,12 +1198,11 @@ struct arm_smmu_cd *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, > struct arm_smmu_l1_ctx_desc *l1_desc; > struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table; > > - if (!cd_table->cdtab) > + if (!arm_smmu_cdtab_allocated(cd_table)) > return NULL; > > if (cd_table->s1fmt == STRTAB_STE_0_S1FMT_LINEAR) > - return (struct arm_smmu_cd *)(cd_table->cdtab + > - ssid * CTXDESC_CD_DWORDS); > + return &cd_table->cdtab.linear[ssid]; > > l1_desc = &cd_table->l1_desc[ssid / CTXDESC_L2_ENTRIES]; > if (!l1_desc->l2ptr) > @@ -1220,7 +1219,7 @@ struct arm_smmu_cd *arm_smmu_alloc_cd_ptr(struct arm_smmu_master *master, > might_sleep(); > iommu_group_mutex_assert(master->dev); > > - if (!cd_table->cdtab) { > + if (!arm_smmu_cdtab_allocated(cd_table)) { > if (arm_smmu_alloc_cd_tables(master)) > return NULL; > } > @@ -1236,7 +1235,7 @@ struct arm_smmu_cd *arm_smmu_alloc_cd_ptr(struct arm_smmu_master *master, > if (arm_smmu_alloc_cd_leaf_table(smmu, l1_desc)) > return NULL; > > - l1ptr = cd_table->cdtab + idx * CTXDESC_L1_DESC_DWORDS; Similarly to patch 1, I believe we can get rid of CTXDESC_L1_DESC_DWORDS entirely. > + l1ptr = &cd_table->cdtab.l1_desc[idx]; > arm_smmu_write_cd_l1_desc(l1ptr, l1_desc); > /* An invalid L1CD can be cached */ > arm_smmu_sync_cd(master, ssid, false); > @@ -1342,7 +1341,7 @@ void arm_smmu_clear_cd(struct arm_smmu_master *master, ioasid_t ssid) > struct arm_smmu_cd target = {}; > struct arm_smmu_cd *cdptr; > > - if (!master->cd_table.cdtab) > + if (!arm_smmu_cdtab_allocated(&master->cd_table)) > return; > cdptr = arm_smmu_get_cd_ptr(master, ssid); > if (WARN_ON(!cdptr)) > @@ -1352,8 +1351,6 @@ void arm_smmu_clear_cd(struct arm_smmu_master *master, ioasid_t ssid) > > static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master) > { > - int ret; > - size_t l1size; > size_t max_contexts; > struct arm_smmu_device *smmu = master->smmu; > struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table; > @@ -1366,8 +1363,14 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master) > cd_table->s1fmt = STRTAB_STE_0_S1FMT_LINEAR; > cd_table->num_l1_ents = max_contexts; > > - l1size = max_contexts * (CTXDESC_CD_DWORDS << 3); > + cd_table->cdtab.linear = dmam_alloc_coherent( > + smmu->dev, max_contexts * sizeof(struct arm_smmu_cd), > + &cd_table->cdtab_dma, GFP_KERNEL); > + if (!cd_table->cdtab.linear) > + return -ENOMEM; > } else { > + size_t l1size; > + > cd_table->s1fmt = STRTAB_STE_0_S1FMT_64K_L2; > cd_table->num_l1_ents = DIV_ROUND_UP(max_contexts, > CTXDESC_L2_ENTRIES); > @@ -1379,24 +1382,15 @@ static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master) > return -ENOMEM; > > l1size = cd_table->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3); > + cd_table->cdtab.l1_desc = dmam_alloc_coherent( > + smmu->dev, l1size, &cd_table->cdtab_dma, GFP_KERNEL); > + if (!cd_table->cdtab.l1_desc) { > + devm_kfree(smmu->dev, cd_table->l1_desc); > + cd_table->l1_desc = NULL; > + return -ENOMEM; > + } > } > - > - cd_table->cdtab = dmam_alloc_coherent(smmu->dev, l1size, &cd_table->cdtab_dma, > - GFP_KERNEL); > - if (!cd_table->cdtab) { > - dev_warn(smmu->dev, "failed to allocate context descriptor\n"); > - ret = -ENOMEM; > - goto err_free_l1; > - } > - > return 0; > - > -err_free_l1: > - if (cd_table->l1_desc) { > - devm_kfree(smmu->dev, cd_table->l1_desc); > - cd_table->l1_desc = NULL; > - } > - return ret; > } > > static void arm_smmu_free_cd_tables(struct arm_smmu_master *master) > @@ -1407,7 +1401,7 @@ static void arm_smmu_free_cd_tables(struct arm_smmu_master *master) > struct arm_smmu_ctx_desc_cfg *cd_table = &master->cd_table; > > if (cd_table->l1_desc) { > - size = CTXDESC_L2_ENTRIES * (CTXDESC_CD_DWORDS << 3); > + size = CTXDESC_L2_ENTRIES * sizeof(struct arm_smmu_cd); > > for (i = 0; i < cd_table->num_l1_ents; i++) { > if (!cd_table->l1_desc[i].l2ptr) > @@ -1421,13 +1415,17 @@ static void arm_smmu_free_cd_tables(struct arm_smmu_master *master) > cd_table->l1_desc = NULL; > > l1size = cd_table->num_l1_ents * (CTXDESC_L1_DESC_DWORDS << 3); > + dmam_free_coherent(smmu->dev, l1size, cd_table->cdtab.l1_desc, > + cd_table->cdtab_dma); > } else { > - l1size = cd_table->num_l1_ents * (CTXDESC_CD_DWORDS << 3); > + dmam_free_coherent(smmu->dev, > + cd_table->num_l1_ents * > + sizeof(struct arm_smmu_cd), > + cd_table->cdtab.linear, cd_table->cdtab_dma); > } > > - dmam_free_coherent(smmu->dev, l1size, cd_table->cdtab, cd_table->cdtab_dma); > cd_table->cdtab_dma = 0; > - cd_table->cdtab = NULL; > + cd_table->cdtab.linear = NULL; nit: AFAIU, arm_smmu_cdtab_allocated() was added to check the cd table existence regardless the config. So, I think we should be consistent, as here it is set as linear, while it can be 2lvl. > } > > bool arm_smmu_free_asid(struct arm_smmu_ctx_desc *cd) > @@ -2955,7 +2953,7 @@ static void arm_smmu_release_device(struct device *dev) > > arm_smmu_disable_pasid(master); > arm_smmu_remove_master(master); > - if (master->cd_table.cdtab) > + if (arm_smmu_cdtab_allocated(&master->cd_table)) > arm_smmu_free_cd_tables(master); > kfree(master); > } > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > index 280a04bfb7230c..21c1acf34dd29c 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h > @@ -279,10 +279,8 @@ struct arm_smmu_ste { > #define CTXDESC_L1_DESC_V (1UL << 0) > #define CTXDESC_L1_DESC_L2PTR_MASK GENMASK_ULL(51, 12) > > -#define CTXDESC_CD_DWORDS 8 > - > struct arm_smmu_cd { > - __le64 data[CTXDESC_CD_DWORDS]; > + __le64 data[8]; > }; > > #define CTXDESC_CD_0_TCR_T0SZ GENMASK_ULL(5, 0) > @@ -312,7 +310,7 @@ struct arm_smmu_cd { > * When the SMMU only supports linear context descriptor tables, pick a > * reasonable size limit (64kB). > */ > -#define CTXDESC_LINEAR_CDMAX ilog2(SZ_64K / (CTXDESC_CD_DWORDS << 3)) > +#define CTXDESC_LINEAR_CDMAX ilog2(SZ_64K / sizeof(struct arm_smmu_cd)) > > /* Command queue */ > #define CMDQ_ENT_SZ_SHIFT 4 > @@ -593,7 +591,10 @@ struct arm_smmu_l1_ctx_desc { > }; > > struct arm_smmu_ctx_desc_cfg { > - __le64 *cdtab; > + union { > + struct arm_smmu_cd *linear; > + __le64 *l1_desc; As patch-1 also, I think naming is confusing. > + } cdtab; > dma_addr_t cdtab_dma; > struct arm_smmu_l1_ctx_desc *l1_desc; > unsigned int num_l1_ents; > @@ -602,6 +603,12 @@ struct arm_smmu_ctx_desc_cfg { > u8 s1cdmax; > }; > > +static inline bool > +arm_smmu_cdtab_allocated(struct arm_smmu_ctx_desc_cfg *cd_table) > +{ > + return cd_table->cdtab.linear; > +} > + > struct arm_smmu_s2_cfg { > u16 vmid; > }; > -- > 2.45.2 > Thanks, Mostafa _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel