On Fri, Jan 10, 2025 at 07:06:49AM +0000, Tian, Kevin wrote:
From: Nicolin Chen nicolinc@nvidia.com Sent: Wednesday, January 8, 2025 1:10 AM
+int iommufd_veventq_alloc(struct iommufd_ucmd *ucmd) +{
- struct iommu_veventq_alloc *cmd = ucmd->cmd;
- struct iommufd_veventq *veventq;
- struct iommufd_viommu *viommu;
- int fdno;
- int rc;
- if (cmd->flags || cmd->type == IOMMU_VEVENTQ_TYPE_DEFAULT)
return -EOPNOTSUPP;- viommu = iommufd_get_viommu(ucmd, cmd->viommu_id);
- if (IS_ERR(viommu))
return PTR_ERR(viommu);- if (!viommu->ops || !viommu->ops->supports_veventq ||
!viommu->ops->supports_veventq(cmd->type))return -EOPNOTSUPP;I'm not sure about the necessity of above check. The event queue is just a software struct with a user-specified format for the iommu driver to report viommu event. The struct itself is not constrained by the hardware capability, though I'm not sure a real usage in which a smmu driver wants to report a vtd event. But legitimately an user can create any type of event queues which might just be never used.
Allowing a random type that a driver will never use for reporting doesn't sound to make a lot of sense to me...
That being said, yea..I guess we could drop the limit here, since it isn't going to break anything?
It sounds clearer to do the check when IOPF cap is actually enabled on a device contained in the viommu. At that point check whether a required type eventqueue has been created. If not then fail the iopf enabling.
Hmm, isn't IOPF a different channel?
And a vEVENTQ is per vIOMMU, not necessarily per vDEVICE/device..
Then it reveals probably another todo in this series. Seems you still let the smmu driver statically enable iopf when probing the device. Sounds like iommufd_viommu_alloc_hwpt_nested() may accept IOMMU_HWPT_FAULT_ID_VALID to refer to a event queue and later dynamically enable/disable iopf when attaching a device to the hwpt and check the event queue type there. Just like how the fault object is handled.
You've lost me here :-/
Thanks Nicolin