Re: [PATCH v6 04/10] lib/igt_drm_ras_nl: add get_error_counter support

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

Hi Soham,

On 30-10-2026 15:34, Purkait, Soham wrote:
> Hi Ravi,
>
> On 29-10-2026 14:29, Ravi Kishore Koppuravuri wrote:
>> Add netlink request/response handling for DRM_RAS_CMD_GET_ERROR_COUNTER.
>>
>> Signed-off-by: Ravi Kishore Koppuravuri 
>> <ravi.kishore.koppuravuri@intel.com>
>> ---
>> v6:Removed normalize_nl_error() for standard errno in ras_error_cb() and
>>     send_and_recv_nl_msg() (Soham)
>> v5:Renamed library function names prefixed with igt_drm_ras_nl_*
>>     (Riana and Soham)
>>     Normalized libnl error codes to standard errnos (Soham)
>>     Added error messages for error handling functions (Riana)
>>     Changed the error-id max value from UINT32_MAX to 
>> DRM_XE_RAS_ERR_COMP_MAX (Riana)
>> v4:Added support to poll for incoming netlink messages with 10 
>> seconds timeout
>>     Renamed the library functions with prefix "igt_drm_netlink_*" and
>>     other cosmetic changes (Nikula Jani & Kamil)
>> v2:Handled ACK/FINISH/ERR nl responses using separate callbacks
>>     Added a check whether error_value updated by valid callback or not
>>     Added a check whether response required for the command or not
>> ---
>>   lib/igt_drm_ras_nl.c | 317 ++++++++++++++++++++++++++++++++++++++++++-
>>   lib/igt_drm_ras_nl.h |  34 +++++
>>   2 files changed, 349 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/igt_drm_ras_nl.c b/lib/igt_drm_ras_nl.c
>> index 2822361d3..7e6a61f11 100644
>> --- a/lib/igt_drm_ras_nl.c
>> +++ b/lib/igt_drm_ras_nl.c
>> @@ -3,14 +3,286 @@
>>    * Copyright © 2026 Intel Corporation
>>    */
>>   +#include <errno.h>
>> +#include <poll.h>
>>   #include <stdint.h>
>>   +#include <netlink/errno.h>
>>   #include <netlink/genl/ctrl.h>
>>   #include <netlink/genl/genl.h>
>>     #include "igt_core.h"
>>   #include "igt_drm_ras_nl.h"
>>   +#define DRM_RAS_NETLINK_REPLY_TIMEOUT_MS 10000
>> +
>> +/* Normalize netlink error codes to standard errnos */
>> +static int normalize_nl_error(int ret)
>> +{
>> +    if (ret >= 0)
>> +        return ret;
>> +
>> +    switch (-ret) {
>> +    case NLE_INVAL:
>> +        return -EINVAL;
>> +    case NLE_NOMEM:
>> +        return -ENOMEM;
>> +    case NLE_NOATTR:
>> +    case NLE_MISSING_ATTR:
>> +        return -ENODATA;
>> +    case NLE_OPNOTSUPP:
>> +        return -EOPNOTSUPP;
>> +    case NLE_EXIST:
>> +        return -EEXIST;
>> +    case NLE_NOADDR:
>> +        return -EADDRNOTAVAIL;
>> +    case NLE_PERM:
>> +        return -EPERM;
>> +    case NLE_NOACCESS:
>> +        return -EACCES;
>> +    case NLE_NODEV:
>> +        return -ENODEV;
>> +    case NLE_BUSY:
>> +        return -EBUSY;
>> +    case NLE_AGAIN:
>> +        return -EAGAIN;
>> +    default:
> Any igt_debug log required? could be useful during debugging.
ok sure. Will add igt_debug()
>> +        return -EIO;
>> +    }
>> +}
>> +
>> +static int ras_command_cb(struct nl_msg *msg, void *arg)
>> +{
>> +    struct igt_drm_ras_nl_context *ctx = arg;
>> +    struct nlmsghdr *nlh;
>> +    struct genlmsghdr *gnlh;
>> +    struct nlattr *attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX + 1];
>> +    int ret;
>> +
>> +    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)
> Any igt_debug log required?
May not be useful. This if() case is hit during the scenarios (1)when 
the message cannot be parsed, (2) lack of expected error-value 
attribute, or (3)unrecognized command. In these cases, SKIPs the current 
case and continues with the later responses for checking valid reply or 
ACK to complete the command.
>> +        return NL_SKIP;
>> +
>> +    switch (gnlh->cmd) {
>> +    case DRM_RAS_CMD_GET_ERROR_COUNTER: {
>> +        if (!attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_VALUE])
>> +            return NL_SKIP;
>> +
>> +        ctx->error_value = 
>> nla_get_u32(attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_VALUE]);
>> +        break;
>> +    }
>> +    default:
> Any igt_debug log required?
Same here. if the command received is not the valid one, it hits default 
case and skips the iteration and continues to the next.
>> +        return NL_SKIP;
>> +    }
>> +
>> +    ctx->reply_received = true;
>> +    if (ctx->ack_received)
>> +        ctx->cmd_done = true;
>> +
>> +    return NL_OK;
>> +}
>> +
>> +static int ras_error_cb(struct sockaddr_nl *nla, struct nlmsgerr 
>> *err, void *arg)
>> +{
>> +    struct igt_drm_ras_nl_context *ctx = arg;
>> +
>> +    /* err->error is a raw kernel errno, not a libnl NLE_* code */
>> +    ctx->last_nl_error = err->error;
>> +    ctx->cmd_done = true;
>> +
>> +    return NL_STOP;
>> +}
>> +
>> +static int ras_ack_cb(struct nl_msg *msg, void *arg)
>> +{
>> +    struct igt_drm_ras_nl_context *ctx = arg;
>> +
>> +    ctx->ack_received = true;
>> +    if (!ctx->response_required || ctx->reply_received)
>> +        ctx->cmd_done = true;
>> +
>> +    return NL_STOP;
>> +}
>> +
>> +static int ras_finish_cb(struct nl_msg *msg, void *arg)
>> +{
>> +    struct igt_drm_ras_nl_context *ctx = arg;
>> +
>> +    ctx->cmd_done = true;
>> +
>> +    return NL_STOP;
>> +}
>> +
>> +static int register_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_ACK, NL_CB_CUSTOM, ras_ack_cb, ctx);
>> +    if (ret < 0) {
>> +        igt_warn("Failed to set NL_CB_ACK callback (ret=%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    ret = nl_cb_set(cb, NL_CB_FINISH, NL_CB_CUSTOM, ras_finish_cb, 
>> ctx);
>> +    if (ret < 0) {
>> +        igt_warn("Failed to set NL_CB_FINISH callback (ret=%d)\n", 
>> ret);
>> +        return ret;
>> +    }
>> +
>> +    ret = nl_cb_err(cb, NL_CB_CUSTOM, ras_error_cb, ctx);
>> +    if (ret < 0) {
>> +        igt_warn("Failed to set NL_CB_ERR 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)
>> +{
>> +    struct pollfd pfd = {
>> +        .fd = nl_socket_get_fd(ctx->sock),
>> +        .events = POLLIN,
>> +    };
>> +    int ret;
>> +
>> +    ret = nl_send_auto(ctx->sock, msg);
>> +    nlmsg_free(msg);
>> +    if (ret < 0) {
>> +        nl_cb_put(cb);
>> +        return normalize_nl_error(ret);
>> +    }
>> +
>> +    /* NL Reply and ACK arrive as separate responses */
>> +    while (!ctx->cmd_done) {
> Is timeout required ?
timeout is added for poll() which waits for 10 secs as part of 
do-while() loop to get a message and outer while() loop is to wait until 
the response is completed / finished. In addition to the per-message 
timeout of 10 secs, will add 1 min overall timeout for the outer while 
loop.
>> +        do {
>> +            /* poll for incoming netlink messages with 10 seconds 
>> timeout */
>> +            ret = poll(&pfd, 1, DRM_RAS_NETLINK_REPLY_TIMEOUT_MS);
>> +        } while (ret < 0 && errno == EINTR);
>> +
>> +        /* poll()/errno results are already standard errnos, not 
>> libnl codes */
>> +        if (ret == 0) {
>> +            nl_cb_put(cb);
>> +            return -ETIMEDOUT;
>> +        }
>> +
>> +        if (ret < 0) {
>> +            nl_cb_put(cb);
>> +            return -errno;
>> +        }
>> +
>> +        if (!(pfd.revents & POLLIN)) {
>> +            nl_cb_put(cb);
> Any igt_debug log required?
will add igt_debug log
>> +            return -EIO;
>> +        }
>> +
>> +        ret = nl_recvmsgs(ctx->sock, cb);
>> +        if (ret < 0) {
>> +            nl_cb_put(cb);
>> +            return normalize_nl_error(ret);
>> +        }
>> +    }
>> +    nl_cb_put(cb);
> nl_cb_put(cb) cleanup is repeated on every exit path. Could a single 
> goto unwind be used ?
sure. will create a goto for this
>> +
>> +    return 0;
>> +}
>> +
>> +static int send_command(struct igt_drm_ras_nl_context *ctx, uint8_t 
>> cmd)
>> +{
>> +    struct nl_cb *cb;
>> +    struct nl_msg *msg;
>> +    void *msg_head;
>> +    int ret;
>> +
>> +    msg = nlmsg_alloc();
>> +    if (!msg)
>> +        return -ENOMEM;
>> +
>> +    ctx->last_nl_error = 0;
>> +    ctx->cmd_done = false;
>> +    ctx->reply_received = false;
>> +    ctx->ack_received = false;
>> +    ctx->response_required = false;
>> +
>> +    msg_head = genlmsg_put(msg,
>> +                   NL_AUTO_PORT,
>> +                   NL_AUTO_SEQ,
>> +                   ctx->family_id,
>> +                   0,
>> +                   NLM_F_REQUEST | NLM_F_ACK,
>> +                   cmd,
>> +                   DRM_RAS_FAMILY_VERSION);
>> +    if (!msg_head) {
>> +        nlmsg_free(msg);
>> +        return -ENOMEM;
>> +    }
>> +
>> +    switch (cmd) {
>> +    case DRM_RAS_CMD_GET_ERROR_COUNTER:
>> +        ctx->response_required = true;
>> +        ret = nla_put_u32(msg,
>> +                  DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID,
>> +                  ctx->node_id);
>> +        if (ret < 0) {
>> +            nlmsg_free(msg);
>> +            return normalize_nl_error(ret);
>> +        }
>> +
>> +        ret = nla_put_u32(msg,
>> +                  DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID,
>> +                  ctx->error_id);
>> +        if (ret < 0) {
>> +            nlmsg_free(msg);
>> +            return normalize_nl_error(ret);
>> +        }
>> +        break;
>> +    default:
>> +        nlmsg_free(msg);
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>> +    cb = nl_cb_alloc(NL_CB_DEFAULT);
>> +    if (!cb) {
>> +        nlmsg_free(msg);
>> +        return -ENOMEM;
>> +    }
>> +
>> +    ret = register_callbacks(cb, ctx);
>> +    if (ret < 0) {
>> +        nl_cb_put(cb);
>> +        nlmsg_free(msg);
>> +        return normalize_nl_error(ret);
>> +    }
>> +
>> +    return send_and_recv_nl_msg(ctx, cb, msg);
>> +}
>> +
>> +static void reset_drm_ras_nl_context(struct igt_drm_ras_nl_context 
>> *ctx)
>> +{
>> +    ctx->sock = NULL;
>> +    ctx->node_id = UINT32_MAX;
>> +    ctx->error_id = DRM_XE_RAS_ERR_COMP_MAX;
>> +    ctx->error_value = 0;
>> +    ctx->last_nl_error = 0;
>> +    ctx->cmd_done = false;
>> +    ctx->reply_received = false;
>> +    ctx->ack_received = false;
>> +    ctx->response_required = false;
>> +    ctx->family_id = -1;
>> +}
>> +
>>   void igt_drm_ras_nl_cleanup(struct igt_drm_ras_nl_context *ctx)
>>   {
>>       if (!ctx || !ctx->sock)
>> @@ -18,14 +290,19 @@ void igt_drm_ras_nl_cleanup(struct 
>> igt_drm_ras_nl_context *ctx)
>>         nl_close(ctx->sock);
>>       nl_socket_free(ctx->sock);
>> -    ctx->sock = NULL;
>> -    ctx->family_id = -1;
>> +
>> +    reset_drm_ras_nl_context(ctx);
>>         igt_debug("Cleaned up netlink socket.\n");
>>   }
>>     int igt_drm_ras_nl_init(struct igt_drm_ras_nl_context *ctx)
> Several paths return a generic -1. Could specific errnos be returned 
> with a igt_warn() ?
will check it once.
>>   {
>> +    if (!ctx)
>> +        return -EINVAL;
>> +
>> +    reset_drm_ras_nl_context(ctx);
>> +
>>       ctx->sock = nl_socket_alloc();
>>       if (!ctx->sock)
>>           return -1;
>> @@ -48,3 +325,39 @@ int igt_drm_ras_nl_init(struct 
>> igt_drm_ras_nl_context *ctx)
>>             DRM_RAS_FAMILY_NAME, ctx->family_id);
>>       return 0;
>>   }
>> +
>> +int igt_drm_ras_nl_get_error_counter(struct igt_drm_ras_nl_context 
>> *ctx)
>> +{
>> +    int ret;
>> +
>> +    if (!ctx || !ctx->sock || ctx->family_id < 0) {
>> +        igt_warn("Invalid DRM RAS netlink context\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    if (ctx->node_id == UINT32_MAX ||
>> +        ctx->error_id == DRM_XE_RAS_ERR_COMP_MAX ||
>> +        ctx->error_id == 0) {
>> +        igt_warn("Invalid node_id (%u) or error_id (%u) provided\n",
>> +             ctx->node_id, ctx->error_id);
>> +        return -EINVAL;
>> +    }
>> +
>> +    ctx->error_value = UINT32_MAX;
> Keep initial value consistent with the reset initial value (0) in 
> reset_drm_ras_nl_context().

reset_drm_ras_nl_context() runs during initialization and cleanup but 
not before every counter query. Setting error_value to UINT32_MAX there 
will not prevent a later query from retaining the previous query’s 
value. If we initialize error_value to 0, we will not be able to 
distinguish whether the kernel returned a valid counter value of zero or 
no counter value was received. Initializing it to UINT32_MAX helps to 
identify the “no reply received” case.

>> +
>> +    ret = send_command(ctx, DRM_RAS_CMD_GET_ERROR_COUNTER);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    if (ctx->error_value == UINT32_MAX) {
>> +        igt_warn("No valid error counter reply: node_id=%u 
>> error_id=%u kernel_error=%d\n",
>> +             ctx->node_id, ctx->error_id, ctx->last_nl_error);
>> +
>> +        return ctx->last_nl_error ? ctx->last_nl_error : -ENODATA;
>
> is last_nl_error always negative ?
>
> Regards,
> Soham

this holds the last error reported by kernel and it will be negative.

Thanks,

Ravi Kishore K.

>
>> +    }
>> +
>> +    igt_debug("Retrieved error counter: node_id=%u error_id=%u 
>> value=%u\n",
>> +          ctx->node_id, ctx->error_id, ctx->error_value);
>> +
>> +    return 0;
>> +}
>> diff --git a/lib/igt_drm_ras_nl.h b/lib/igt_drm_ras_nl.h
>> index 011f3bc05..f51784524 100644
>> --- a/lib/igt_drm_ras_nl.h
>> +++ b/lib/igt_drm_ras_nl.h
>> @@ -7,14 +7,48 @@
>>   #define IGT_DRM_RAS_NL_H
>>     #include <drm-uapi/drm_ras.h>
>> +#include <drm-uapi/xe_drm.h>
>>   +/**
>> + * struct igt_drm_ras_nl_context - DRM RAS generic netlink context
>> + *
>> + * Holds the state for a DRM RAS generic netlink request/response 
>> exchange.
>> + */
>>   struct igt_drm_ras_nl_context {
>> +    /** @sock: Netlink socket used for the exchange */
>>       struct nl_sock *sock;
>> +
>> +    /** @node_id: DRM RAS node ID to query the error counters */
>> +    uint32_t node_id;
>> +
>> +    /** @error_id: RAS error ID to query the error counter */
>> +    uint32_t error_id;
>> +
>> +    /** @error_value: Error counter value returned by the kernel */
>> +    uint32_t error_value;
>> +
>> +    /** @last_nl_error: Last netlink error reported by the kernel */
>> +    int last_nl_error;
>> +
>> +    /** @cmd_done: Set when the command exchange has completed */
>> +    bool cmd_done;
>> +
>> +    /** @reply_received: Set when the command reply has been 
>> received */
>> +    bool reply_received;
>> +
>> +    /** @ack_received: Set when the command ACK has been received */
>> +    bool ack_received;
>> +
>> +    /** @response_required: Set when a reply is expected before 
>> @cmd_done */
>> +    bool response_required;
>> +
>> +    /** @family_id: Generic netlink family ID */
>>       int family_id;
>>   };
>>     void igt_drm_ras_nl_cleanup(struct igt_drm_ras_nl_context *ctx);
>>   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);
>>     #endif /* IGT_DRM_RAS_NL_H */