Re: [PATCH v6 07/25] iommu/mtk_iommu_v1: Implement an IDENTITY domain

Baolu Lu
2023-08-14T03:07:12Z

On 2023/8/3 8:07, Jason Gunthorpe wrote:
> What mtk does during mtk_iommu_v1_set_platform_dma() is actually putting
> the iommu into identity mode. Make this available as a proper IDENTITY
> domain.
>
> The mtk_iommu_v1_def_domain_type() from
> commit 8bbe13f52cb7 ("iommu/mediatek-v1: Add def_domain_type") explains
> this was needed to allow probe_finalize() to be called, but now the
> IDENTITY domain will do the same job so change the returned
> def_domain_type.
>
> mkt_v1 is the only driver that returns IOMMU_DOMAIN_UNMANAGED from
> def_domain_type(). This allows the next patch to enforce an IDENTITY
> domain policy for this driver.

This code seems to be not working properly.

* @def_domain_type: device default domain type, return value:
* - IOMMU_DOMAIN_IDENTITY: must use an identity domain
* - IOMMU_DOMAIN_DMA: must use a dma domain
* - 0: use the default setting

The core code is not ready to accept any other return value.

>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
> drivers/iommu/mtk_iommu_v1.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index 8a0a5e5d049f4a..cc3e7d53d33ad9 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c
> @@ -319,11 +319,27 @@ static int mtk_iommu_v1_attach_device(struct iommu_domain *domain, struct device
> return 0;
> }
>
> -static void mtk_iommu_v1_set_platform_dma(struct device *dev)
> +static int mtk_iommu_v1_identity_attach(struct iommu_domain *identity_domain,
> + struct device *dev)
> {
> struct mtk_iommu_v1_data *data = dev_iommu_priv_get(dev);
>
> mtk_iommu_v1_config(data, dev, false);
> + return 0;
> +}
> +
> +static struct iommu_domain_ops mtk_iommu_v1_identity_ops = {
> + .attach_dev = mtk_iommu_v1_identity_attach,
> +};
> +
> +static struct iommu_domain mtk_iommu_v1_identity_domain = {
> + .type = IOMMU_DOMAIN_IDENTITY,
> + .ops = &mtk_iommu_v1_identity_ops,
> +};
> +
> +static void mtk_iommu_v1_set_platform_dma(struct device *dev)
> +{
> + mtk_iommu_v1_identity_attach(&mtk_iommu_v1_identity_domain, dev);

This callback seems to be a dead code now.

> }
>
> static int mtk_iommu_v1_map(struct iommu_domain *domain, unsigned long iova,
> @@ -443,7 +459,7 @@ static int mtk_iommu_v1_create_mapping(struct device *dev, struct of_phandle_arg
>
> static int mtk_iommu_v1_def_domain_type(struct device *dev)
> {
> - return IOMMU_DOMAIN_UNMANAGED;
> + return IOMMU_DOMAIN_IDENTITY;

def_domain_type can't be used for this purpose. But this seems to be a
temporary code, as it will be removed in patch 09/25.

> }
>
> static struct iommu_device *mtk_iommu_v1_probe_device(struct device *dev)
> @@ -578,6 +594,7 @@ static int mtk_iommu_v1_hw_init(const struct mtk_iommu_v1_data *data)
> }
>
> static const struct iommu_ops mtk_iommu_v1_ops = {
> + .identity_domain = &mtk_iommu_v1_identity_domain,
> .domain_alloc = mtk_iommu_v1_domain_alloc,
> .probe_device = mtk_iommu_v1_probe_device,
> .probe_finalize = mtk_iommu_v1_probe_finalize,

Best regards,
baolu