On Fri, May 30, 2025 at 01:07:53PM -0300, Jason Gunthorpe wrote:
> On Sat, May 17, 2025 at 08:21:30PM -0700, Nicolin Chen wrote:
> > struct iommufd_viommu_ops {
> > + u32 flags;
> > void (*destroy)(struct iommufd_viommu *viommu);
> > struct iommu_domain *(*alloc_domain_nested)(
> > struct iommufd_viommu *viommu, u32 flags,
> > @@ -171,6 +200,10 @@ struct iommufd_viommu_ops {
> > struct device *dev,
> > u64 virt_id);
> > void (*vdevice_destroy)(struct iommufd_vdevice *vdev);
> > + struct iommufd_hw_queue *(*hw_queue_alloc)(
> > + struct iommufd_ucmd *ucmd, struct iommufd_viommu *viommu,
> > + unsigned int type, u32 index, u64 base_addr, size_t length);
>
> I think it would better to have two function pointers here than the flags:
>
> + struct iommufd_hw_queue *(*hw_queue_alloc)(
> + struct iommufd_ucmd *ucmd, struct iommufd_viommu *viommu,
> + unsigned int type, u32 index, u64 s2_iova, size_t length);
>
>
> + struct iommufd_hw_queue *(*hw_queue_alloc_phys)(
> + struct iommufd_ucmd *ucmd, struct iommufd_viommu *viommu,
> + unsigned int type, u32 index, phys_addr_t phys, size_t length);
OK. I think these two should be exclusive then. Maybe it needs a
WARN_ON in iommufd_viommu_alloc.
Thanks
Nicolin
