From: Nicolin Chen nicolinc@nvidia.com Sent: Saturday, January 11, 2025 5:29 AM
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?
We have a fault queue for delivering IOPF on hwpt, when vIOMMU is not involved
Now with vIOMMU my understanding was that all events including IOPF are delivered via the event queue in the vIOMMU. Just echoed by the documentation patch:
+- IOMMUFD_OBJ_VEVENTQ, representing a software queue for a vIOMMU to report its + events such as translation faults occurred to a nested stage-1 and HW-specific + events.
And a vEVENTQ is per vIOMMU, not necessarily per vDEVICE/device..
Yes. My point was to verify whether the vEVENTQ type is compatible when a nested faultable hwpt is created with vIOMMU as the parent. then when attaching a device to the nested hwpt we dynamically turn on PRI on the device just like how it's handled in the fault queue path.
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 :-/
Hope above explanation makes my point clearer. Then for a nested hwpt created within a vIOMMU there is an open whether we want a per-hwpt option to mark whether it allows fault, or assume that every nested hwpt (and the devices attached to it) must be faultable once any vEVENTQ is created in the vIOMMU.