On Tue, Feb 18, 2025 at 05:13:47AM +0000, Tian, Kevin wrote:
From: Nicolin Chen nicolinc@nvidia.com Sent: Saturday, January 25, 2025 8:31 AM
+/*
- An iommufd_veventq object represents an interface to deliver vIOMMU
events to
- the user space. It is created/destroyed by the user space and associated
with
- vIOMMU object(s) during the allocations.
s/object(s)/object/, given the eventq cannot be shared between vIOMMUs.
Done. Adding an "a" too.
+static inline void iommufd_vevent_handler(struct iommufd_veventq *veventq,
struct iommufd_vevent *vevent)+{
- struct iommufd_eventq *eventq = &veventq->common;
- /*
* Remove the overflow node and add the new node at the sametime. Note
* it is possible that vevent == &veventq->overflow for sequenceupdate
*/- spin_lock(&eventq->lock);
- if (veventq->overflow.on_list) {
list_del(&veventq->overflow.node);veventq->overflow.on_list = false;- }
We can save one field 'on_list' in every entry by:
if (list_is_last(&veventq->overflow.node, &eventq->deliver)) list_del(&veventq->overflow.node);
Hmm. Given that the overflow node, if being on the list, should be always the last one... yes!
+struct iommufd_vevent_header {
- __aligned_u64 flags;
- __u32 sequence;
- __u32 __reserved;
+};
Is there a reason that flags must be u64? At a glance all flags fields (except the one in iommu_hwpt_vtd_s1) in iommufd uAPIs are u32 which can cut the size of the header by half...
Not having a particular reason yet. Just thought that a 64-bit could make the uAPI more expandable. It's true that u32 would be cleaner. I will make a change.
+void iommufd_veventq_abort(struct iommufd_object *obj) +{
- struct iommufd_eventq *eventq =
container_of(obj, struct iommufd_eventq, obj);- struct iommufd_veventq *veventq = eventq_to_veventq(eventq);
- struct iommufd_viommu *viommu = veventq->viommu;
- struct iommufd_vevent *cur, *next;
- lockdep_assert_held_write(&viommu->veventqs_rwsem);
- list_for_each_entry_safe(cur, next, &eventq->deliver, node) {
list_del(&cur->node);kfree(cur);kfree() doesn't apply to the overflow node.
Oh right, that's missed.
otherwise it looks good to me:
Reviewed-by: Kevin Tian kevin.tian@intel.com
Thanks! Nicolin