> On 24 Jan 2022, at 13:33, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> On Mon, Jan 24, 2022 at 12:25:32PM +0000, Haakon Bugge wrote:
>>
>>
>>> On 23 Jan 2022, at 11:34, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>>>
>>> On Wed, Jan 19, 2022 at 04:28:09AM -0500, mike.marciniszyn@xxxxxxxxxxxxxxxxxxxx wrote:
>>>> From: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxxxxxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxxxxxxxxxxxx>
>>>> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxxxxxxxxxxxxx>
>>>> ---
>>>> 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?
For hfi1, I do not know. But this fix was in drivers/infiniband/sw/rdmavt, for which the first commit message states:
This patch introduces the basics for a new module called rdma_vt. This new
driver is a software implementation of the InfiniBand verbs...
More importantly, the check we discuss is not about being page-aligned, but about being naturally aligned, right?
Thxs, Håkon
>
> 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
