On 2018/3/15 4:30, Jason Gunthorpe wrote:
> On Mon, Mar 05, 2018 at 09:53:05PM +0800, Yixian Liu wrote:
>> diff --git a/providers/hns/hns_roce_u_abi.h b/providers/hns/hns_roce_u_abi.h
>> index ec145bb..c4f36ec 100644
>> +++ b/providers/hns/hns_roce_u_abi.h
>> @@ -57,6 +57,7 @@ struct hns_roce_create_cq_resp {
>> struct ib_uverbs_create_cq_resp ibv_resp;
>> __u32 cqn;
>> __u32 reserved;
>> + __u64 cap_flags;
>> };
>
> This struct is wrong.. the kernel does this:
>
> if (context) {
> if ((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB) &&
> (udata->outlen == sizeof(resp))) {
> hr_cq->db_en = 1;
> resp.cqn = hr_cq->cqn;
> resp.cap_flags |= HNS_ROCE_SUPPORT_CQ_RECORD_DB;
> ret = ib_copy_to_udata(udata, &resp, sizeof(resp));
> } else
> ret = ib_copy_to_udata(udata, &hr_cq->cqn, sizeof(u64));
>
> So 'cqn' should be a u64, it is a mistake in the userspace to have
> used a u32 here.
>
> You could correct it, but it would need to be used as u32
> consistently.
>
> Instead I recommend changing the above struct (in user and kernel) to
> simply:
>
> struct hns_roce_ib_create_cq_resp {
> __u64 cqn; /* Only 32 bits used, 64 for compat */
> __u64 cap_flags;
> };
>
> And change the kernel to simply do:
>
> if (context) {
> resp.cqn = hr_cq->cqn;
> if ((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB) &&
> (udata->outlen == sizeof(resp))) {
> hr_cq->db_en = 1;
> resp.cap_flags |= HNS_ROCE_SUPPORT_CQ_RECORD_DB;
> }
> ret = ib_copy_to_udata(udata, &resp, sizeof(resp));
>
> ib_copy_to_udata() will do the right thing.
>
> Please send a patch for this immediately before this gets into a
> released kernel..
>
> Also all these tests:
>
> if ((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB) &&
> (udata->outlen == sizeof(resp)))
>
> Should probably be:
>
> if ((hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RECORD_DB) &&
> (udata->outlen >= sizeof(resp)))
>
> Arguably a flag should have been added to the request for this new
> behavior though, as is will limit what you can do in future.
>
> Jason
Thank you very much! I will send the patch sets for kernel and userspace today.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
