Re: [PATCH for-rc] IB/rdmavt: Validate remote_addr during loopback atomic tests

[Leon Romanovsky <leon@kernel.org>]

On Mon, Jan 24, 2022 at 12:25:32PM +0000, Haakon Bugge wrote:
> 
> 
> > On 23 Jan 2022, at 11:34, Leon Romanovsky <leon@kernel.org> wrote:
> > 
> > On Wed, Jan 19, 2022 at 04:28:09AM -0500, mike.marciniszyn@cornelisnetworks.com wrote:
> >> From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
> >> 
> >> The rdma-core test suite sends an unaligned remote address
> >> and expects a failure.
> >> 
> >> ERROR: test_atomic_non_aligned_addr (tests.test_atomic.AtomicTest)
> >> 
> >> The qib/hfi1 rc handling validates properly, but the test has the
> >> client and server on the same system.
> >> 
> >> The loopback of these operations is a distinct code path.
> >> 
> >> Fix by syntaxing the proposed remote address in the loopback
> >> code path.
> >> 
> >> Fixes: 15703461533a ("IB/{hfi1, qib, rdmavt}: Move ruc_loopback to rdmavt")
> >> Reviewed-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
> >> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>
> >> ---
> >> drivers/infiniband/sw/rdmavt/qp.c | 2 ++
> >> 1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
> >> index 3305f27..ae50b56 100644
> >> --- a/drivers/infiniband/sw/rdmavt/qp.c
> >> +++ b/drivers/infiniband/sw/rdmavt/qp.c
> >> @@ -3073,6 +3073,8 @@ void rvt_ruc_loopback(struct rvt_qp *sqp)
> >> 	case IB_WR_ATOMIC_FETCH_AND_ADD:
> >> 		if (unlikely(!(qp->qp_access_flags & IB_ACCESS_REMOTE_ATOMIC)))
> >> 			goto inv_err;
> >> +		if (unlikely(wqe->atomic_wr.remote_addr & (sizeof(u64) - 1)))
> > 
> > Isn't this "!PAGE_ALIGNED(wqe->atomic_wr.remote_addr)" check?
> 
> No, it checks that the address is natural aligned, in this case the three LSBs must be zero. As per IBTA:
> 
> <quote>
> The virtual address in the ATOMIC Command Request packet shall be naturally aligned to an 8 byte boundary.
> </quote>

And is IBTA restriction applicable to hfi1?

Thanks
> 
> 
> Thxs, Håkon
> 
> > 
> > Thanks
> > 
> >> +			goto inv_err;
> >> 		if (unlikely(!rvt_rkey_ok(qp, &qp->r_sge.sge, sizeof(u64),
> >> 					  wqe->atomic_wr.remote_addr,
> >> 					  wqe->atomic_wr.rkey,
> >> -- 
> >> 1.8.3.1
>