Re: [PATCH for-next v3 2/2] RDMA/efa: Pass relaxed ordering flag to device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] efa: missing firmware capability check for relaxed ordering
--

commit 8cde16309300638ef5ed5570f8cb4ae347e5c6e6
Author: Dana Malachi <danamala@amazon.com>

RDMA/efa: Pass relaxed ordering flag to device

This commit accepts the relaxed ordering access flag during memory region
registration and forwards it to the device firmware via the admin command
path.

> diff --git a/drivers/infiniband/hw/efa/efa_com_cmd.c b/drivers/infiniband/hw/efa/efa_com_cmd.c
> index aac0e936601a1..07bfcb093df85 100644
> --- a/drivers/infiniband/hw/efa/efa_com_cmd.c
> +++ b/drivers/infiniband/hw/efa/efa_com_cmd.c
[ ... ]
> @@ -236,6 +236,8 @@ int efa_com_register_mr(struct efa_com_dev *edev,
>  	mr_cmd.mr_length = params->mr_length_in_bytes;
>  	EFA_SET(&mr_cmd.flags, EFA_ADMIN_REG_MR_CMD_PHYS_PAGE_SIZE_SHIFT,
>  		params->page_shift);
> +	if (params->relaxed_ordering)
> +		EFA_SET(&mr_cmd.flags, EFA_ADMIN_REG_MR_CMD_RELAXED_ORDERING_ENABLE, 1);

[Severity: Medium]
Could setting this bit cause memory region registration to fail on older
firmware versions?

If older firmware strictly enforces the MBZ (Must Be Zero) rule for bit 6,
it might reject the command when userspace requests relaxed ordering. Since
there doesn't appear to be a check against device attributes or firmware
capabilities here, does the driver need to verify firmware support before
passing this flag down in efa_com_register_mr()?

[ ... ]
> diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
> index 6356872753507..9e4435401a4c7 100644
> --- a/drivers/infiniband/hw/efa/efa_verbs.c
> +++ b/drivers/infiniband/hw/efa/efa_verbs.c
[ ... ]
> @@ -1679,6 +1679,7 @@ static int efa_register_mr(struct ib_pd *ibpd, struct efa_mr *mr, u64 start,
>  	params.permissions.local_write = !!(access_flags & IB_ACCESS_LOCAL_WRITE);
>  	params.permissions.remote_write = !!(access_flags & IB_ACCESS_REMOTE_WRITE);
>  	params.permissions.remote_read = !!(access_flags & IB_ACCESS_REMOTE_READ);
> +	params.relaxed_ordering = !!(access_flags & IB_ACCESS_RELAXED_ORDERING);

[Severity: Medium]
This passes the userspace IB_ACCESS_RELAXED_ORDERING flag directly into
the command parameters in efa_register_mr(). 

Should this be gated by a capability check so we don't inadvertently break
MR registration for older devices if a userspace application unconditionally
sets this access flag?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922160118.990-1-mrgolin@amazon.com?part=2