Re: [PATCH v4 03/13] iommu/arm-smmu-v3: Refactor write_strtab_ent

From: Nicolin Chen <nicolinc@nvidia.com>
To: Michael Shavit <mshavit@google.com>
Cc: Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	"Joerg Roedel" <joro@8bytes.org>, <jean-philippe@linaro.org>,
	<jgg@nvidia.com>, <baolu.lu@linux.intel.com>,
	<linux-arm-kernel@lists.infradead.org>, <iommu@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 03/13] iommu/arm-smmu-v3: Refactor write_strtab_ent
Date: Wed, 12 Jul 2023 18:41:46 -0700	[thread overview]
Message-ID: <ZK9WWilcF6GwqHiu@Asurada-Nvidia> (raw)
In-Reply-To: <20230621063825.268890-4-mshavit@google.com>

On Wed, Jun 21, 2023 at 02:37:15PM +0800, Michael Shavit wrote:

> Explicity keep track of the s1_cfg and s2_cfg that are attached to a
> master in arm_smmu_master, regardless of whether they are owned by
> arm_smmu_master, arm_smmu_domain or userspace.

An s1_cfg is in a master while an s2_cfg is in a domain. So we
we know where they are. I kinda get that having these two ptrs
could ease the cleanup, especially in arm_smmu_write_strtab_ent.

> 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 053cc14c23969..3c614fbe2b8b9 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -689,6 +689,8 @@ struct arm_smmu_master {
>         struct list_head                domain_head;
>         struct arm_smmu_stream          *streams;
>         struct arm_smmu_s1_cfg          owned_s1_cfg;
> +       struct arm_smmu_s1_cfg          *s1_cfg;
> +       struct arm_smmu_s2_cfg          *s2_cfg;

Yet, this looks a bit overcomplicated to me by having an s1_cfg
that points to master->owned_s1_cfg?

I am wondering if it'd be neater to have a new struct for STE,
replacing the owned_s1_cfg, s1_cfg and s2_cfg above. It could
be something like struct arm_smmu_cmdq_ent, which contains all
the STE fields so that in arm_smmu_write_strtab_ent we can just
plainly copy to the dst[]. Also, whenever a device attaches to
a domain having the necessary info needed by the STE, we update
the STE struct owned by the master.

Thanks
Nic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel