[3/3] common/mlx5: fix DevX register access opcode
Checks
Commit Message
The dedicated MLX5_CMD_OP_ACCESS_REGISTER_USER opcode
must be used to read hardware register content from
unprotected mode.
Fixes: 737f44a25d97 ("common/mlx5: add register access DevX routine")
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
drivers/common/mlx5/mlx5_devx_cmds.c | 3 ++-
drivers/common/mlx5/mlx5_prm.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
Comments
17/07/2020 16:28, Viacheslav Ovsiienko:
> The dedicated MLX5_CMD_OP_ACCESS_REGISTER_USER opcode
> must be used to read hardware register content from
> unprotected mode.
Otherwise? What was broken?
>
> Fixes: 737f44a25d97 ("common/mlx5: add register access DevX routine")
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, July 17, 2020 18:06
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh <rasland@mellanox.com>
> Subject: Re: [PATCH 3/3] common/mlx5: fix DevX register access opcode
>
> 17/07/2020 16:28, Viacheslav Ovsiienko:
> > The dedicated MLX5_CMD_OP_ACCESS_REGISTER_USER opcode must be
> used to
> > read hardware register content from unprotected mode.
>
> Otherwise? What was broken?
Otherwise the MLX5_CMD_OP_ACCESS_REGISTER was used, it returned EINVAL
and register value was not read. It was supposed to enable ACCESS_REGISTER
operation from user mode in kernel driver to read registers, but eventually
it was replaced with ACCESS_REGISTER_USER dedicated operation.
mlx5 PMD does not rely on this feature strongly, if register reading fails
it deduces the timestamp mode from reported timestamp counter
frequency.
With best regards, Slava
17/07/2020 17:11, Slava Ovsiienko:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 17/07/2020 16:28, Viacheslav Ovsiienko:
> > > The dedicated MLX5_CMD_OP_ACCESS_REGISTER_USER opcode must be
> > used to
> > > read hardware register content from unprotected mode.
> >
> > Otherwise? What was broken?
>
> Otherwise the MLX5_CMD_OP_ACCESS_REGISTER was used, it returned EINVAL
> and register value was not read. It was supposed to enable ACCESS_REGISTER
> operation from user mode in kernel driver to read registers, but eventually
> it was replaced with ACCESS_REGISTER_USER dedicated operation.
>
> mlx5 PMD does not rely on this feature strongly, if register reading fails
> it deduces the timestamp mode from reported timestamp counter
> frequency.
OK I think some of these explanations deserve to be in the commit log.
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, July 17, 2020 18:19
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh <rasland@mellanox.com>
> Subject: Re: [PATCH 3/3] common/mlx5: fix DevX register access opcode
>
> 17/07/2020 17:11, Slava Ovsiienko:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 17/07/2020 16:28, Viacheslav Ovsiienko:
> > > > The dedicated MLX5_CMD_OP_ACCESS_REGISTER_USER opcode must
> be
> > > used to
> > > > read hardware register content from unprotected mode.
> > >
> > > Otherwise? What was broken?
> >
> > Otherwise the MLX5_CMD_OP_ACCESS_REGISTER was used, it returned
> > EINVAL and register value was not read. It was supposed to enable
> > ACCESS_REGISTER operation from user mode in kernel driver to read
> > registers, but eventually it was replaced with ACCESS_REGISTER_USER
> dedicated operation.
> >
> > mlx5 PMD does not rely on this feature strongly, if register reading
> > fails it deduces the timestamp mode from reported timestamp counter
> > frequency.
>
> OK I think some of these explanations deserve to be in the commit log.
M-m-m-m, this is quite tiny 2-lines fix, we are going to squash it before rc2 😊
OK, will add the comment about dedicated opcode.
With best regards, Slava
17/07/2020 17:23, Slava Ovsiienko:
>
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Friday, July 17, 2020 18:19
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> > Darawsheh <rasland@mellanox.com>
> > Subject: Re: [PATCH 3/3] common/mlx5: fix DevX register access opcode
> >
> > 17/07/2020 17:11, Slava Ovsiienko:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 17/07/2020 16:28, Viacheslav Ovsiienko:
> > > > > The dedicated MLX5_CMD_OP_ACCESS_REGISTER_USER opcode must
> > be
> > > > used to
> > > > > read hardware register content from unprotected mode.
> > > >
> > > > Otherwise? What was broken?
> > >
> > > Otherwise the MLX5_CMD_OP_ACCESS_REGISTER was used, it returned
> > > EINVAL and register value was not read. It was supposed to enable
> > > ACCESS_REGISTER operation from user mode in kernel driver to read
> > > registers, but eventually it was replaced with ACCESS_REGISTER_USER
> > dedicated operation.
> > >
> > > mlx5 PMD does not rely on this feature strongly, if register reading
> > > fails it deduces the timestamp mode from reported timestamp counter
> > > frequency.
> >
> > OK I think some of these explanations deserve to be in the commit log.
>
> M-m-m-m, this is quite tiny 2-lines fix, we are going to squash it before rc2 😊
> OK, will add the comment about dedicated opcode.
If the patch is squashed with the root cause, no need to explain indeed.
@@ -44,7 +44,8 @@
DRV_LOG(ERR, "Not enough buffer for register read data");
return -1;
}
- MLX5_SET(access_register_in, in, opcode, MLX5_CMD_OP_ACCESS_REGISTER);
+ MLX5_SET(access_register_in, in, opcode,
+ MLX5_CMD_OP_ACCESS_REGISTER_USER);
MLX5_SET(access_register_in, in, op_mod,
MLX5_ACCESS_REGISTER_IN_OP_MOD_READ);
MLX5_SET(access_register_in, in, register_id, reg_id);
@@ -806,6 +806,7 @@ enum {
MLX5_CMD_OP_CREATE_GENERAL_OBJECT = 0xa00,
MLX5_CMD_OP_MODIFY_GENERAL_OBJECT = 0xa01,
MLX5_CMD_OP_QUERY_GENERAL_OBJECT = 0xa02,
+ MLX5_CMD_OP_ACCESS_REGISTER_USER = 0xB0C,
};
enum {