Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <e73f3a0e-5c1b-674e-7ef1-53d963a540d3@linux.intel.com>
Date:   Wed, 27 Sep 2023 10:17:31 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Cc:     baolu.lu@linux.intel.com, cohuck@redhat.com, eric.auger@redhat.com,
        nicolinc@nvidia.com, kvm@vger.kernel.org, mjrosato@linux.ibm.com,
        chao.p.peng@linux.intel.com, yi.y.sun@linux.intel.com,
        peterx@redhat.com, jasowang@redhat.com,
        shameerali.kolothum.thodi@huawei.com, lulu@redhat.com,
        suravee.suthikulpanit@amd.com, iommu@lists.linux.dev,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        zhenzhong.duan@intel.com, joao.m.martins@oracle.com
Subject: Re: [RFC 2/8] iommufd: replace attach_fn with a structure
Content-Language: en-US
To:     Yi Liu <yi.l.liu@intel.com>, joro@8bytes.org,
        alex.williamson@redhat.com, jgg@nvidia.com, kevin.tian@intel.com,
        robin.murphy@arm.com
References: <20230926092651.17041-1-yi.l.liu@intel.com>
 <20230926092651.17041-3-yi.l.liu@intel.com>
From:   Baolu Lu <baolu.lu@linux.intel.com>
In-Reply-To: <20230926092651.17041-3-yi.l.liu@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/26/23 5:26 PM, Yi Liu wrote:
> Most of the core logic before conducting the actual device attach/
> replace operation can be shared with pasid attach/replace. Create
> a new structure so more information (e.g. pasid) can be later added
> along with the attach_fn.
> 
> Signed-off-by: Kevin Tian<kevin.tian@intel.com>
> Signed-off-by: Yi Liu<yi.l.liu@intel.com>
> ---
>   drivers/iommu/iommufd/device.c          | 35 ++++++++++++++++---------
>   drivers/iommu/iommufd/iommufd_private.h |  8 ++++++
>   2 files changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 645ab5d290fe..4fa4153c5df7 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -597,8 +597,11 @@ iommufd_device_do_replace(struct iommufd_device *idev,
>   	return ERR_PTR(rc);
>   }
>   
> -typedef struct iommufd_hw_pagetable *(*attach_fn)(
> -	struct iommufd_device *idev, struct iommufd_hw_pagetable *hwpt);
> +static struct iommufd_hw_pagetable *do_attach(struct iommufd_device *idev,
> +		struct iommufd_hw_pagetable *hwpt, struct attach_data *data)
> +{
> +	return data->attach_fn(idev, hwpt);
> +}

I assume that this change was made because we need to pass the pasid
value to the attach_fn() callback.

If so, how about passing it directly to attach_fn() function?

typedef struct iommufd_hw_pagetable *(*attach_fn)(
		struct iommufd_device *idev,
		struct iommufd_hw_pagetable *hwpt,
		ioasid_t pasid);

In no pasid case, use IOMMU_NO_PASID.

Best regards,
baolu