>-----Original Message-----
>From: Shameer Kolothum Thodi <[email protected]>
>Subject: RE: [RFC PATCH 01/14] backends/iommufd: Add pasid attach/detach
>callbacks
>
>
>
>> -----Original Message-----
>> From: Zhenzhong Duan <[email protected]>
>> Subject: [RFC PATCH 01/14] backends/iommufd: Add pasid attach/detach
>> callbacks
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Add two wrappers host_iommu_device_iommufd_pasid_[at|de]tach_hwpt to
>> wrap the two callbacks.
>>
>> Use assert to ensure the corresponding callbacks exist.
>>
>> Signed-off-by: Yi Liu <[email protected]>
>> Signed-off-by: Zhenzhong Duan <[email protected]>
>> ---
>> include/system/iommufd.h | 40
>> ++++++++++++++++++++++++++++++++++++++++
>> backends/iommufd.c | 23 +++++++++++++++++++++++
>> 2 files changed, 63 insertions(+)
>>
>> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
>> index 80d72469a9..615be10aed 100644
>> --- a/include/system/iommufd.h
>> +++ b/include/system/iommufd.h
>> @@ -145,10 +145,50 @@ struct HostIOMMUDeviceIOMMUFDClass {
>> * Returns: true on success, false on failure.
>> */
>> bool (*detach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, Error **errp);
>> + /**
>> + * @attach_hwpt: attach host IOMMU device's pasid to IOMMUFD
>
>s/@attach_hwpt/@pasid_attach_hwpt/
Oh, will fix, missed it after copy/paste😊
>
>> hardware page
>> + * table. VFIO and VDPA device can have different implementation.
>> + *
>> + * Mandatory callback.
>
>This currently appears Intel specific and calling it "Mandatory"
>may be confusing.
Same here, will fix.
> Have you considered extending attach_hwpt() to
>take a PASID instead of introducing a separate callback?
Yes, I did. To extending attach/detach_hwpt(), we need to add two
parameters, "bool has_pasid, uint32_t pasid". E.g.,
bool (*attach_hwpt)(HostIOMMUDeviceIOMMUFD *idev,
bool has_pasid, uint32_t pasid,
uint32_t hwpt_id, Error **errp);
and refactor iommufd_cdev_attach/detach_ioas_hwpt() a bit.
I'm fine either way, two separate callbacks just look cleaner for me.
Let me know if extending is preferred.
>
>> + *
>> + * @idev: host IOMMU device backed by IOMMUFD backend.
>> + *
>> + * @pasid: pasid of host IOMMU device.
>> + *
>> + * @hwpt_id: ID of IOMMUFD hardware page table.
>> + *
>> + * @errp: pass an Error out when attachment fails.
>> + *
>> + * Returns: true on success, false on failure.
>> + */
>> + bool (*pasid_attach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, uint32_t
>> pasid,
>> + uint32_t hwpt_id, Error **errp);
>> + /**
>> + * @detach_hwpt: detach host IOMMU device's from IOMMUFD hardware
>
>s/@detach_hwpt/@pasid_detach_hwpt/
>
>Also comment needs update?
Yes, will fix that.
Thanks
Zhenzhong
