On Mon, Jan 16, 2023 at 05:56:01PM -0600, Bob Pearson wrote:
> +/* only implemented for 64 bit architectures */
> +int rxe_mr_do_atomic_write(struct rxe_mr *mr, u64 iova, u64 value)
> +{
> +#if defined CONFIG_64BIT
#ifdef
It is a little more typical style to provide an alternate version of
the function when #ifdefing
> + u64 *va;
> +
> + /* See IBA oA19-28 */
> + if (unlikely(mr->state != RXE_MR_STATE_VALID)) {
> + rxe_dbg_mr(mr, "mr not in valid state");
> + return -EINVAL;
> + }
> +
> + va = iova_to_vaddr(mr, iova, sizeof(value));
> + if (unlikely(!va)) {
> + rxe_dbg_mr(mr, "iova out of range");
> + return -ERANGE;
> + }
> +
> + /* See IBA A19.4.2 */
> + if (unlikely((uintptr_t)va & 0x7 || iova & 0x7)) {
> + rxe_dbg_mr(mr, "misaligned address");
> + return -RXE_ERR_NOT_ALIGNED;
> + }
> +
> + /* Do atomic write after all prior operations have completed */
> + smp_store_release(va, value);
> +
> + return 0;
> +#else
> + WARN_ON(1);
> + return -EINVAL;
> +#endif
> +}
> +
> int advance_dma_data(struct rxe_dma_info *dma, unsigned int length)
> {
> struct rxe_sge *sge = &dma->sge[dma->cur_sge];
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index 1e38e5da1f4c..49298ff88d25 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -764,30 +764,40 @@ static enum resp_states atomic_reply(struct rxe_qp *qp,
> return RESPST_ACKNOWLEDGE;
> }
>
> -#ifdef CONFIG_64BIT
> -static enum resp_states do_atomic_write(struct rxe_qp *qp,
> - struct rxe_pkt_info *pkt)
> +static enum resp_states atomic_write_reply(struct rxe_qp *qp,
> + struct rxe_pkt_info *pkt)
> {
> - struct rxe_mr *mr = qp->resp.mr;
> - int payload = payload_size(pkt);
> - u64 src, *dst;
> -
> - if (mr->state != RXE_MR_STATE_VALID)
> - return RESPST_ERR_RKEY_VIOLATION;
> + struct resp_res *res = qp->resp.res;
> + struct rxe_mr *mr;
> + u64 value;
> + u64 iova;
> + int err;
>
> - memcpy(&src, payload_addr(pkt), payload);
> + if (!res) {
> + res = rxe_prepare_res(qp, pkt, RXE_ATOMIC_WRITE_MASK);
> + qp->resp.res = res;
> + }
>
> - dst = iova_to_vaddr(mr, qp->resp.va + qp->resp.offset, payload);
> - /* check vaddr is 8 bytes aligned. */
> - if (!dst || (uintptr_t)dst & 7)
> - return RESPST_ERR_MISALIGNED_ATOMIC;
> + if (res->replay)
> + return RESPST_ACKNOWLEDGE;
>
> - /* Do atomic write after all prior operations have completed */
> - smp_store_release(dst, src);
> + mr = qp->resp.mr;
> + value = *(u64 *)payload_addr(pkt);
> + iova = qp->resp.va + qp->resp.offset;
>
> - /* decrease resp.resid to zero */
> - qp->resp.resid -= sizeof(payload);
> +#if defined CONFIG_64BIT
Shouldn't need a #ifdef here
> + err = rxe_mr_do_atomic_write(mr, iova, value);
> + if (unlikely(err)) {
> + if (err == -RXE_ERR_NOT_ALIGNED)
> + return RESPST_ERR_MISALIGNED_ATOMIC;
> + else
> + return RESPST_ERR_RKEY_VIOLATION;
Again why not return the RESPST directly then the stub can return
RESPST_ERR_UNSUPPORTED_OPCODE?
Jason
