Re: [PATCH v6 08/10] lib/igt_drm_ras_nl: add event notify subscription and event wait support

["Koppuravuri, Ravi Kishore" <ravi.kishore.koppuravuri@intel.com>]

Hi Soham,

On 30-10-2026 17:34, Purkait, Soham wrote:
> Hi Ravi,
>
> On 29-10-2026 14:29, Ravi Kishore Koppuravuri wrote:
>> Added event notification support with subscribe for an error-notify 
>> event
>> and wait for the event notification functionalities.
>>
>> Signed-off-by: Ravi Kishore Koppuravuri 
>> <ravi.kishore.koppuravuri@intel.com>
>> ---
>> v6:Added normalize_nl_error() for subscribe and wait_for_event functions
>> v5:Added a guard to prevent duplicate multicast group subscriptions 
>> (Soham)
>> v4:Renamed the library functions with prefix "igt_drm_netlink_*" (Nikula
>>     Jani & Kamil)
>>     updated error event name from error-notify to error-report to align
>>     with the latest changes in drm-uapi/drm_ras.h
>> v2:Handled SEQ_CHECK nl response using separate callback
>>     Added monotonic-clock deadline to wait for messages without 
>> exceeding the
>>     caller’s total timeout.
>>     Added support to receive events until it sees an event whose node 
>> ID and
>>     error ID match the requested values.
>> ---
>>   lib/igt_drm_ras_nl.c | 184 +++++++++++++++++++++++++++++++++++++++++--
>>   lib/igt_drm_ras_nl.h |  18 +++++
>>   2 files changed, 195 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/igt_drm_ras_nl.c b/lib/igt_drm_ras_nl.c
>> index d23fce08f..62e48662e 100644
>> --- a/lib/igt_drm_ras_nl.c
>> +++ b/lib/igt_drm_ras_nl.c
>> @@ -6,6 +6,7 @@
>>   #include <errno.h>
>>   #include <poll.h>
>>   #include <stdint.h>
>> +#include <time.h>
>>     #include <netlink/errno.h>
>>   #include <netlink/genl/ctrl.h>
>> @@ -62,10 +63,12 @@ static int ras_command_cb(struct nl_msg *msg, 
>> void *arg)
>>       nlh = nlmsg_hdr(msg);
>>       gnlh = nlmsg_data(nlh);
>>   -    ret = genlmsg_parse(nlh, 0, attrs,
>> -                DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX, NULL);
>> -    if (ret < 0)
>> -        return NL_SKIP;
>> +    if (gnlh->cmd != DRM_RAS_CMD_ERROR_EVENT) {
>> +        ret = genlmsg_parse(nlh, 0, attrs,
>> +                    DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX, NULL);
>> +        if (ret < 0)
>> +            return NL_SKIP;
>> +    }
>>         switch (gnlh->cmd) {
>>       case DRM_RAS_CMD_SET_ERROR_THRESHOLD:
>> @@ -85,14 +88,44 @@ static int ras_command_cb(struct nl_msg *msg, 
>> void *arg)
>> nla_get_u32(attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD]);
>>           break;
>>       }
>> +    case DRM_RAS_CMD_ERROR_EVENT: {
>> +        struct nlattr *event_attrs[DRM_RAS_A_ERROR_EVENT_ATTRS_MAX + 
>> 1];
>> +
>> +        ret = genlmsg_parse(nlh, 0, event_attrs,
>> +                    DRM_RAS_A_ERROR_EVENT_ATTRS_MAX, NULL);
>> +        if (ret < 0)
>> +            return NL_SKIP;
>> +
>> +        if (!event_attrs[DRM_RAS_A_ERROR_EVENT_ATTRS_NODE_ID] ||
>> +            !event_attrs[DRM_RAS_A_ERROR_EVENT_ATTRS_ERROR_ID] ||
>> + !event_attrs[DRM_RAS_A_ERROR_EVENT_ATTRS_ERROR_VALUE])
>> +            return NL_SKIP;
>> +
>> +        ctx->event_node_id =
>> + nla_get_u32(event_attrs[DRM_RAS_A_ERROR_EVENT_ATTRS_NODE_ID]);
>> +        ctx->event_error_id =
>> + nla_get_u32(event_attrs[DRM_RAS_A_ERROR_EVENT_ATTRS_ERROR_ID]);
>> +        ctx->event_error_value =
>> + nla_get_u32(event_attrs[DRM_RAS_A_ERROR_EVENT_ATTRS_ERROR_VALUE]);
>> +        ctx->event_received = ctx->event_node_id == ctx->node_id &&
>> +                      ctx->event_error_id == ctx->error_id;
>> +        break;
>> +    }
>>       default:
>>           return NL_SKIP;
>>       }
>>   -    ctx->reply_received = true;
>> -    if (ctx->ack_received)
>> -        ctx->cmd_done = true;
>> +    if (gnlh->cmd != DRM_RAS_CMD_ERROR_EVENT) {
>> +        ctx->reply_received = true;
>> +        if (ctx->ack_received)
>> +            ctx->cmd_done = true;
>> +    }
>> +
>> +    return NL_OK;
>> +}
>>   +static int ras_multicast_seq_check_cb(struct nl_msg *msg, void *arg)
>> +{
>>       return NL_OK;
>>   }
>>   @@ -158,6 +191,27 @@ static int register_callbacks(struct nl_cb 
>> *cb, struct igt_drm_ras_nl_context *c
>>       return 0;
>>   }
>>   +static int register_multicast_callbacks(struct nl_cb *cb,
>> +                    struct igt_drm_ras_nl_context *ctx)
>> +{
>> +    int ret;
>> +
>> +    ret = nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, ras_command_cb, 
>> ctx);
>> +    if (ret < 0) {
>> +        igt_warn("Failed to set NL_CB_VALID callback (ret=%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    ret = nl_cb_set(cb, NL_CB_SEQ_CHECK, NL_CB_CUSTOM,
>> +            ras_multicast_seq_check_cb, ctx);
>> +    if (ret < 0) {
>> +        igt_warn("Failed to set NL_CB_SEQ_CHECK callback 
>> (ret=%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int send_and_recv_nl_msg(struct igt_drm_ras_nl_context *ctx,
>>                   struct nl_cb *cb,
>>                   struct nl_msg *msg)
>> @@ -303,7 +357,12 @@ static void reset_drm_ras_nl_context(struct 
>> igt_drm_ras_nl_context *ctx)
>>       ctx->ack_received = false;
>>       ctx->response_required = false;
>>       ctx->error_threshold = 0;
>> +    ctx->event_node_id = UINT32_MAX;
>> +    ctx->event_error_id = UINT32_MAX;
>> +    ctx->event_error_value = 0;
>> +    ctx->event_received = false;
>>       ctx->family_id = -1;
>> +    ctx->subscribed_grp_id = -1;
>>   }
>>     void igt_drm_ras_nl_cleanup(struct igt_drm_ras_nl_context *ctx)
>> @@ -446,3 +505,114 @@ int igt_drm_ras_nl_set_error_threshold(struct 
>> igt_drm_ras_nl_context *ctx)
>>         return 0;
>>   }
>> +
>> +int igt_drm_ras_nl_subscribe_error_report(struct 
>> igt_drm_ras_nl_context *ctx,
>> +                      const char *notify_group_name)
>> +{
>> +    int grp_id;
>> +    int ret;
>> +
>> +    if (!ctx || !ctx->sock || ctx->family_id < 0 ||
>> +        !notify_group_name || notify_group_name[0] == '\0')
> Could a short igt_debug()/igt_warn() be added ?
will add
>> +        return -EINVAL;
>> +
>> +    grp_id = genl_ctrl_resolve_grp(ctx->sock,
>> +                       DRM_RAS_FAMILY_NAME,
>> +                       notify_group_name);
>> +    if (grp_id < 0)
>> +        return normalize_nl_error(grp_id);
>> +
>> +    /* Check if already subscribed to the requested multicast group */
>> +    if (ctx->subscribed_grp_id == grp_id) {
>> +        igt_debug("Already subscribed to DRM RAS multicast group 
>> '%s' (id=%d).\n",
>> +              notify_group_name, grp_id);
>> +        return 0;
>> +    }
>> +
>> +    ret = nl_socket_add_membership(ctx->sock, grp_id);
>> +    if (ret < 0)
>> +        return normalize_nl_error(ret);
>> +
>> +    ctx->subscribed_grp_id = grp_id;
>> +
>> +    igt_debug("Subscribed to DRM RAS multicast group '%s' (id=%d).\n",
>> +          notify_group_name, grp_id);
>> +
>> +    return 0;
>> +}
>> +
>> +int igt_drm_ras_nl_wait_for_error_report_event(struct 
>> igt_drm_ras_nl_context *ctx,
>> +                           int timeout_ms)
>> +{
>> +    struct nl_cb *cb;
>> +    struct pollfd pfd;
>> +    struct timespec start, now;
>> +    int remaining_ms;
>> +    int ret;
> Inverted xmas tree.
will update
>> +
>> +    if (!ctx || !ctx->sock || ctx->family_id < 0)
>> +        return -EINVAL;
>> +
>> +    if (timeout_ms < -1)
>> +        return -EINVAL;
>> +
>> +    cb = nl_cb_alloc(NL_CB_DEFAULT);
>> +    if (!cb)
> igt_warn/igt_debug log
Here, as it is returning a specific system error, IMO debug log is not 
needed.
>> +        return -ENOMEM;
>> +
>> +    ret = register_multicast_callbacks(cb, ctx);
>> +    if (ret < 0) {
>> +        nl_cb_put(cb);
>> +        return normalize_nl_error(ret);
>> +    }
>> +
>> +    ctx->event_received = false;
>> +    clock_gettime(CLOCK_MONOTONIC, &start);
>> +
>> +    pfd.fd = nl_socket_get_fd(ctx->sock);
>> +    pfd.events = POLLIN;
>> +    pfd.revents = 0;
>> +
>> +    while (!ctx->event_received) {
>> +        if (timeout_ms == -1) {
>> +            remaining_ms = -1;
>> +        } else {
>> +            clock_gettime(CLOCK_MONOTONIC, &now);
>> +            remaining_ms = timeout_ms -
>> +                       (int)((now.tv_sec - start.tv_sec) * 1000 +
>> +                       (now.tv_nsec - start.tv_nsec) / 1000000);
>> +            if (remaining_ms <= 0) {
> igt_warn/igt_debug log
will add
>> +                ret = -ETIMEDOUT;
>> +                break;
>> +            }
>> +        }
>> +
>> +        ret = poll(&pfd, 1, remaining_ms);
>> +        if (ret == 0) {
> ditto
will add
>> +            ret = -ETIMEDOUT;
>> +            break;
>> +        }
>> +
>> +        if (ret < 0) {
>> +            ret = -errno;
>> +            break;
>> +        }
>> +
>> +        ret = nl_recvmsgs(ctx->sock, cb);
>> +        if (ret < 0) {
>> +            ret = normalize_nl_error(ret);
>> +            break;
>> +        }
>> +    }
>> +    nl_cb_put(cb);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    if (!ctx->event_received)
>> +        return -ENOMSG;
>> +
>> +    igt_debug("Received error-notify event: node_id=%u error_id=%u 
>> value=%u\n",
> Could the log string be renamed to "error-report" instead of 
> "error-notify" to stay consistent with the function name?
Updated. Thanks
>
> Regards,
> Soham
>> +          ctx->event_node_id, ctx->event_error_id, 
>> ctx->event_error_value);
>> +
>> +    return 0;
>> +}
>> diff --git a/lib/igt_drm_ras_nl.h b/lib/igt_drm_ras_nl.h
>> index 5c9be5b17..057c6026f 100644
>> --- a/lib/igt_drm_ras_nl.h
>> +++ b/lib/igt_drm_ras_nl.h
>> @@ -47,6 +47,21 @@ struct igt_drm_ras_nl_context {
>>         /** @error_threshold: Error threshold value to query/set */
>>       uint32_t error_threshold;
>> +
>> +    /** @event_node_id: RAS node ID reported by an error-report 
>> event */
>> +    uint32_t event_node_id;
>> +
>> +    /** @event_error_id: RAS error ID reported by an error-report 
>> event */
>> +    uint32_t event_error_id;
>> +
>> +    /** @event_error_value: Error counter value reported by an 
>> error-report event */
>> +    uint32_t event_error_value;
>> +
>> +    /** @event_received: Set when a matching error-report event has 
>> been received */
>> +    bool event_received;
>> +
>> +    /** @subscribed_grp_id: Multicast group ID currently subscribed 
>> to, or -1 if none */
>> +    int subscribed_grp_id;
>>   };
>>     void igt_drm_ras_nl_cleanup(struct igt_drm_ras_nl_context *ctx);
>> @@ -54,6 +69,9 @@ int igt_drm_ras_nl_init(struct 
>> igt_drm_ras_nl_context *ctx);
>>   int igt_drm_ras_nl_get_error_counter(struct igt_drm_ras_nl_context 
>> *ctx);
>>   int igt_drm_ras_nl_get_error_threshold(struct 
>> igt_drm_ras_nl_context *ctx);
>>   int igt_drm_ras_nl_set_error_threshold(struct 
>> igt_drm_ras_nl_context *ctx);
>> +int igt_drm_ras_nl_subscribe_error_report(struct 
>> igt_drm_ras_nl_context *ctx,
>> +                      const char *notify_group_name);
>> +int igt_drm_ras_nl_wait_for_error_report_event(struct 
>> igt_drm_ras_nl_context *ctx, int timeout_ms);
>>     #endif /* IGT_DRM_RAS_NL_H */