Re: [PATCH v1 05/14] iommu/arm-smmu-v3: Remove ARM_SMMU_DOMAIN_NESTED

Eric Auger
2023-03-10T16:40:46Z

Hi Nicolin,

On 3/9/23 11:53, Nicolin Chen wrote:
> IOMMUFD designs two iommu_domain pointers to represent two stages. The S1
s/designs/uses?
> iommu_domain (IOMMU_DOMAIN_NESTED type) represents the Context Descriptor
> table in the user space. The S2 iommu_domain (IOMMU_DOMAIN_UNMANAGED type)
> represents the translation table in the kernel, owned by a hypervisor.
>
> So there comes to no use case of the ARM_SMMU_DOMAIN_NESTED. Drop it, and
> use the type IOMMU_DOMAIN_NESTED instead.
last sentence may be rephrased as this patch does not use
IOMMU_DOMAIN_NESTED anywhere:
Generic IOMMU_DOMAIN_NESTED type will be used in nested SMMU
implementation instead.
>
> Also drop the unused arm_smmu_enable_nesting(). One following patche will
> configure the correct smmu_domain->stage.
>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 18 ------------------
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 -
If you go this way you may also remove it from arm/arm-smmu/arm-smmu.c.
Then if I am not wrong no other driver does implement enable_nesting cb.
Shouldn't we also remove it and fellow iommu_enable_nesting()?

Thanks

Eric
> 2 files changed, 19 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 c1aac695ae0d..c5616145e2a3 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1279,7 +1279,6 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
> s1_cfg = &smmu_domain->s1_cfg;
> break;
> case ARM_SMMU_DOMAIN_S2:
> - case ARM_SMMU_DOMAIN_NESTED:
> s2_cfg = &smmu_domain->s2_cfg;
> break;
> default:
> @@ -2220,7 +2219,6 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain,
> fmt = ARM_64_LPAE_S1;
> finalise_stage_fn = arm_smmu_domain_finalise_s1;
> break;
> - case ARM_SMMU_DOMAIN_NESTED:
> case ARM_SMMU_DOMAIN_S2:
> ias = smmu->ias;
> oas = smmu->oas;
> @@ -2747,21 +2745,6 @@ static struct iommu_group *arm_smmu_device_group(struct device *dev)
> return group;
> }
>
> -static int arm_smmu_enable_nesting(struct iommu_domain *domain)
> -{
> - struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> - int ret = 0;
> -
> - mutex_lock(&smmu_domain->init_mutex);
> - if (smmu_domain->smmu)
> - ret = -EPERM;
> - else
> - smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
> - mutex_unlock(&smmu_domain->init_mutex);
> -
> - return ret;
> -}
> -
> static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
> {
> return iommu_fwspec_add_ids(dev, args->args, 1);
> @@ -2890,7 +2873,6 @@ static struct iommu_ops arm_smmu_ops = {
> .flush_iotlb_all = arm_smmu_flush_iotlb_all,
> .iotlb_sync = arm_smmu_iotlb_sync,
> .iova_to_phys = arm_smmu_iova_to_phys,
> - .enable_nesting = arm_smmu_enable_nesting,
> .free = arm_smmu_domain_free,
> }
> };
> 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 ba2b4562f4b2..233bfc377267 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -704,7 +704,6 @@ struct arm_smmu_master {
> enum arm_smmu_domain_stage {
> ARM_SMMU_DOMAIN_S1 = 0,
> ARM_SMMU_DOMAIN_S2,
> - ARM_SMMU_DOMAIN_NESTED,
> ARM_SMMU_DOMAIN_BYPASS,
> };