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

[Nicolin Chen]

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