On 6/3/26 18:33, Srinivasan Shanmugam wrote: > The current EVENTFD manager matches subscriptions only by event > identifier. That is enough for coarse notification, but it does not > support the queue-scoped routing needed by queue completion style > events. > > Extend the subscription key from a single event identifier to the pair > (event_type, queue_id). > > For device/GPU-scoped events, queue_id is 0. For queue-scoped events, > queue_id selects the queue-specific subscription. > > EVENTFD remains notification-only. > > Also fix the queue-aware bind path to compute the packed subscription > key before lookup/insert, and allow fd 0 by rejecting only negative file > descriptors. > > This change keeps the existing manager design and binding model intact, > while making queue_id meaningful for queue-scoped wakeups.
Yeah that doesn't work like this. The queue_id is just an identifier for the UAPI and can be re-used. So you can't put that as key in the amdgpu_eventfd_id structure. I suggest to convert the queue_id into the global doorbell id or put the eventfd manager on the queue itself instead of the fpriv. Regards, Christian. > > Cc: Alex Deucher <[email protected]> > Cc: Christian König <[email protected]> > Signed-off-by: Srinivasan Shanmugam <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.c | 92 ++++++++++++++------- > drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.h | 16 +++- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 20 +++-- > 3 files changed, 86 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.c > index 0b0c9268aedc..fefc85ca916e 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.c > @@ -36,25 +36,39 @@ > > #include <linux/slab.h> > #include <linux/err.h> > +#include <drm/amdgpu_drm.h> > > #include "amdgpu_eventfd.h" > > #define AMDGPU_EVENTFD_MAX_BINDS 4096 > > +static bool amdgpu_eventfd_valid_type(u32 event_type) > +{ > + switch (event_type) { > + case DRM_AMDGPU_EVENT_TYPE_USERQ_EOP: > + case DRM_AMDGPU_EVENT_TYPE_QUEUE_RESET: > + case DRM_AMDGPU_EVENT_TYPE_MEMORY_EXCEPTION: > + case DRM_AMDGPU_EVENT_TYPE_SCRATCH: > + return true; > + default: > + return false; > + } > +} > + > /** > * amdgpu_eventfd_id_alloc - allocate an event id container > - * @event_id: userspace-defined event identifier > + * @key: packed (event_type, queue_id) subscription key > * > - * Each event_id represents a notification category. Multiple eventfds can > - * be bound to the same event_id. > + * Each key represents one subscription category. Multiple eventfds can > + * be bound to the same key. > * > * This function allocates the container which stores the list of eventfds > - * associated with that event_id. > + * associated with that subscription key. > * > * Return: > * Pointer to the newly allocated structure or NULL on failure. > */ > -static struct amdgpu_eventfd_id *amdgpu_eventfd_id_alloc(u32 event_id) > +static struct amdgpu_eventfd_id *amdgpu_eventfd_id_alloc(u64 key) > { > struct amdgpu_eventfd_id *id; > > @@ -62,7 +76,7 @@ static struct amdgpu_eventfd_id > *amdgpu_eventfd_id_alloc(u32 event_id) > if (!id) > return NULL; > > - id->event_id = event_id; > + id->key = key; > INIT_HLIST_HEAD(&id->entries); > id->n_entries = 0; > return id; > @@ -71,9 +85,9 @@ static struct amdgpu_eventfd_id > *amdgpu_eventfd_id_alloc(u32 event_id) > /** > * amdgpu_eventfd_id_get_or_create - find or create an event_id entry > * @mgr: eventfd manager > - * @event_id: event identifier > + * @key: packed (event_type, queue_id) subscription key > * > - * This helper returns the container associated with the given event_id. > + * This helper returns the container associated with the given key. > * If it does not exist, it will create one. > * > * The function is designed to be callable without holding any locks. > @@ -84,7 +98,7 @@ static struct amdgpu_eventfd_id > *amdgpu_eventfd_id_alloc(u32 event_id) > * Pointer to the event_id structure or NULL on failure. > */ > static struct amdgpu_eventfd_id * > -amdgpu_eventfd_id_get_or_create(struct amdgpu_eventfd_mgr *mgr, u32 event_id) > +amdgpu_eventfd_id_get_or_create(struct amdgpu_eventfd_mgr *mgr, u64 key) > { > struct amdgpu_eventfd_id *id; > struct amdgpu_eventfd_id *new_id; > @@ -92,19 +106,19 @@ amdgpu_eventfd_id_get_or_create(struct > amdgpu_eventfd_mgr *mgr, u32 event_id) > int r; > > xa_lock_irqsave(&mgr->ids, flags); > - id = xa_load(&mgr->ids, event_id); > + id = xa_load(&mgr->ids, key); > xa_unlock_irqrestore(&mgr->ids, flags); > if (id) > return id; > > - new_id = amdgpu_eventfd_id_alloc(event_id); > + new_id = amdgpu_eventfd_id_alloc(key); > if (!new_id) > return NULL; > > xa_lock_irqsave(&mgr->ids, flags); > > /* Re-check after taking the lock in case another thread inserted it. */ > - id = xa_load(&mgr->ids, event_id); > + id = xa_load(&mgr->ids, key); > if (id) { > xa_unlock_irqrestore(&mgr->ids, flags); > kfree(new_id); > @@ -115,9 +129,9 @@ amdgpu_eventfd_id_get_or_create(struct amdgpu_eventfd_mgr > *mgr, u32 event_id) > * xa_insert() returns -EBUSY if an entry already exists. > * Since we are in irqsave context here, use GFP_ATOMIC. > */ > - r = xa_insert(&mgr->ids, event_id, new_id, GFP_ATOMIC); > + r = xa_insert(&mgr->ids, key, new_id, GFP_ATOMIC); > if (r == -EBUSY) > - id = xa_load(&mgr->ids, event_id); > + id = xa_load(&mgr->ids, key); > > xa_unlock_irqrestore(&mgr->ids, flags); > > @@ -190,13 +204,14 @@ void amdgpu_eventfd_mgr_fini(struct amdgpu_eventfd_mgr > *mgr) > } > > /** > - * amdgpu_eventfd_bind - bind eventfd to an event_id > + * amdgpu_eventfd_bind - bind eventfd to an EVENTFD subscription > * @mgr: eventfd manager > - * @event_id: userspace event identifier > + * @event_type: kernel-defined event type > + * @queue_id: queue identifier, or 0 for device/GPU-scoped events > * @eventfd: eventfd file descriptor > * > * This function allows userspace to subscribe to notifications for a > - * specific event_id. > + * specific (event_type, queue_id) pair. > * > * Multiple eventfds can be bound to the same event_id. > * > @@ -206,17 +221,21 @@ void amdgpu_eventfd_mgr_fini(struct amdgpu_eventfd_mgr > *mgr) > * Return: > * 0 on success, negative error code on failure. > */ > -int amdgpu_eventfd_bind(struct amdgpu_eventfd_mgr *mgr, u32 event_id, int > eventfd) > +int amdgpu_eventfd_bind(struct amdgpu_eventfd_mgr *mgr, u32 event_type, > + u32 queue_id, int eventfd) > { > struct amdgpu_eventfd_id *id; > struct amdgpu_eventfd_entry *e, *it; > struct eventfd_ctx *ctx; > + u64 key; > unsigned long flags; > bool dup = false; > > - if (!mgr || !event_id || eventfd < 0) > + if (!mgr || !eventfd || eventfd < 0 || > !amdgpu_eventfd_valid_type(event_type)) > return -EINVAL; > > + key = amdgpu_eventfd_key(event_type, queue_id); > + > /* > * Enforce total bind limit without a separate manager lock. > * For duplicate binds, we decrement back before returning success. > @@ -232,7 +251,7 @@ int amdgpu_eventfd_bind(struct amdgpu_eventfd_mgr *mgr, > u32 event_id, int eventf > return PTR_ERR(ctx); > } > > - id = amdgpu_eventfd_id_get_or_create(mgr, event_id); > + id = amdgpu_eventfd_id_get_or_create(mgr, key); > if (!id) { > eventfd_ctx_put(ctx); > atomic_dec(&mgr->bind_count); > @@ -294,9 +313,10 @@ int amdgpu_eventfd_bind(struct amdgpu_eventfd_mgr *mgr, > u32 event_id, int eventf > } > > /** > - * amdgpu_eventfd_unbind - remove eventfd binding > + * amdgpu_eventfd_unbind - remove EVENTFD binding > * @mgr: eventfd manager > - * @event_id: event identifier > + * @event_type: kernel-defined event type > + * @queue_id: queue identifier, or 0 for device/GPU-scoped events > * @eventfd: eventfd file descriptor > * > * Removes an existing binding between an event_id and an eventfd. > @@ -304,25 +324,29 @@ int amdgpu_eventfd_bind(struct amdgpu_eventfd_mgr *mgr, > u32 event_id, int eventf > * Return: > * 0 if removed, -ENOENT if binding does not exist. > */ > -int amdgpu_eventfd_unbind(struct amdgpu_eventfd_mgr *mgr, u32 event_id, int > eventfd) > +int amdgpu_eventfd_unbind(struct amdgpu_eventfd_mgr *mgr, u32 event_type, > + u32 queue_id, int eventfd) > { > struct amdgpu_eventfd_id *id; > + u64 key; > struct amdgpu_eventfd_entry *e; > struct hlist_node *tmp; > struct eventfd_ctx *ctx; > unsigned long flags; > bool removed = false; > > - if (!mgr || !event_id || eventfd < 0) > + if (!mgr || eventfd < 0 || !amdgpu_eventfd_valid_type(event_type)) > return -EINVAL; > > + key = amdgpu_eventfd_key(event_type, queue_id); > + > ctx = eventfd_ctx_fdget(eventfd); > if (IS_ERR(ctx)) > return PTR_ERR(ctx); > > xa_lock_irqsave(&mgr->ids, flags); > > - id = xa_load(&mgr->ids, event_id); > + id = xa_load(&mgr->ids, key); > if (!id) > goto out_unlock; > > @@ -338,7 +362,7 @@ int amdgpu_eventfd_unbind(struct amdgpu_eventfd_mgr *mgr, > u32 event_id, int even > atomic_dec(&mgr->bind_count); > > if (!id->n_entries) { > - __xa_erase(&mgr->ids, event_id); > + __xa_erase(&mgr->ids, key); > kfree(id); > } > break; > @@ -353,31 +377,37 @@ int amdgpu_eventfd_unbind(struct amdgpu_eventfd_mgr > *mgr, u32 event_id, int even > } > > /** > - * amdgpu_eventfd_signal - notify all eventfds bound to event_id > + * amdgpu_eventfd_signal - notify all eventfds bound to subscription key > * @mgr: eventfd manager > * @event_id: event identifier > + * @event_type: kernel-defined event type > + * @queue_id: queue identifier, or 0 for device/GPU-scoped events > * > * This function is typically called from interrupt context. > * > - * All eventfds registered for the given event_id will be signaled. > + * All eventfds registered for the given subscription will be signaled. > * Userspace processes waiting on those eventfds will wake up. > */ > -void amdgpu_eventfd_signal(struct amdgpu_eventfd_mgr *mgr, u32 event_id) > +void amdgpu_eventfd_signal(struct amdgpu_eventfd_mgr *mgr, u32 event_type, > + u32 queue_id) > { > struct amdgpu_eventfd_id *id; > struct amdgpu_eventfd_entry *e; > + u64 key; > unsigned long flags; > > - if (!mgr || !event_id) > + if (!mgr || !amdgpu_eventfd_valid_type(event_type)) > return; > > + key = amdgpu_eventfd_key(event_type, queue_id); > + > /* > * IRQ-safe signaling path: keep xarray lock held while iterating and > * signaling. eventfd_signal() is IRQ-safe. > */ > xa_lock_irqsave(&mgr->ids, flags); > > - id = xa_load(&mgr->ids, event_id); > + id = xa_load(&mgr->ids, key); > if (id) { > hlist_for_each_entry(e, &id->entries, hnode) > eventfd_signal(e->ctx); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.h > index 248afb1f2f14..6e7eb513fbc5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eventfd.h > @@ -38,7 +38,7 @@ struct amdgpu_eventfd_entry { > }; > > struct amdgpu_eventfd_id { > - u32 event_id; > + u64 key; > struct hlist_head entries; > u32 n_entries; > }; > @@ -48,12 +48,20 @@ struct amdgpu_eventfd_mgr { > atomic_t bind_count; /* total binds across all event_ids */ > }; > > +static inline u64 amdgpu_eventfd_key(u32 event_type, u32 queue_id) > +{ > + return ((u64)event_type << 32) | queue_id; > +} > + > void amdgpu_eventfd_mgr_init(struct amdgpu_eventfd_mgr *mgr); > void amdgpu_eventfd_mgr_fini(struct amdgpu_eventfd_mgr *mgr); > > -int amdgpu_eventfd_bind(struct amdgpu_eventfd_mgr *mgr, u32 event_id, int > eventfd); > -int amdgpu_eventfd_unbind(struct amdgpu_eventfd_mgr *mgr, u32 event_id, int > eventfd); > +int amdgpu_eventfd_bind(struct amdgpu_eventfd_mgr *mgr, u32 event_type, > + u32 queue_id, int eventfd); > +int amdgpu_eventfd_unbind(struct amdgpu_eventfd_mgr *mgr, u32 event_type, > + u32 queue_id, int eventfd); > > -void amdgpu_eventfd_signal(struct amdgpu_eventfd_mgr *mgr, u32 event_id); > +void amdgpu_eventfd_signal(struct amdgpu_eventfd_mgr *mgr, u32 event_type, > + u32 queue_id); > > #endif /* __AMDGPU_EVENTFD_H__ */ > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index 75fc3a74db28..22dfd22210c4 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -646,21 +646,27 @@ int amdgpu_eventfd_ioctl(struct drm_device *dev, void > *data, > if (args->flags || !args->event_type || args->eventfd < 0) > return -EINVAL; > > - /* > - * queue_id is reserved for future queue-specific subscriptions. > - * Keep it zero for now. > - */ > - if (args->queue_id) > + switch (args->event_type) { > + case DRM_AMDGPU_EVENT_TYPE_USERQ_EOP: > + case DRM_AMDGPU_EVENT_TYPE_QUEUE_RESET: > + case DRM_AMDGPU_EVENT_TYPE_SCRATCH: > + break; > + case DRM_AMDGPU_EVENT_TYPE_MEMORY_EXCEPTION: > + if (args->queue_id) > + return -EINVAL; > + break; > + default: > return -EINVAL; > + } > > switch (args->op) { > case DRM_AMDGPU_EVENTFD_OP_BIND: > return amdgpu_eventfd_bind(&fpriv->eventfd_mgr, > - args->event_type, > + args->event_type, args->queue_id, > args->eventfd); > case DRM_AMDGPU_EVENTFD_OP_UNBIND: > return amdgpu_eventfd_unbind(&fpriv->eventfd_mgr, > - args->event_type, > + args->event_type, args->queue_id, > args->eventfd); > default: > return -EINVAL;
