From patchwork Thu Aug 2 16:13:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yishai Hadas X-Patchwork-Id: 10553947 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A7DDB13B4 for ; Thu, 2 Aug 2018 16:14:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 95DEA2C2CA for ; Thu, 2 Aug 2018 16:14:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8A0342C2F4; Thu, 2 Aug 2018 16:14:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 969D52C30A for ; Thu, 2 Aug 2018 16:14:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726673AbeHBSGb (ORCPT ); Thu, 2 Aug 2018 14:06:31 -0400 Received: from mail-il-dmz.mellanox.com ([193.47.165.129]:48800 "EHLO mellanox.co.il" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726636AbeHBSGb (ORCPT ); Thu, 2 Aug 2018 14:06:31 -0400 Received: from Internal Mail-Server by MTLPINE1 (envelope-from yishaih@mellanox.com) with ESMTPS (AES256-SHA encrypted); 2 Aug 2018 19:17:58 +0300 Received: from vnc17.mtl.labs.mlnx (vnc17.mtl.labs.mlnx [10.7.2.17]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id w72GEXi1029950; Thu, 2 Aug 2018 19:14:33 +0300 Received: from vnc17.mtl.labs.mlnx (vnc17.mtl.labs.mlnx [127.0.0.1]) by vnc17.mtl.labs.mlnx (8.13.8/8.13.8) with ESMTP id w72GEXIw019386; Thu, 2 Aug 2018 19:14:33 +0300 Received: (from yishaih@localhost) by vnc17.mtl.labs.mlnx (8.13.8/8.13.8/Submit) id w72GEXSp019383; Thu, 2 Aug 2018 19:14:33 +0300 From: Yishai Hadas To: linux-rdma@vger.kernel.org Cc: yishaih@mellanox.com, jgg@mellanox.com, majd@mellanox.com Subject: [PATCH rdma-core 1/6] verbs: Use the new kabi macros with the write fallback system Date: Thu, 2 Aug 2018 19:13:31 +0300 Message-Id: <1533226416-19122-2-git-send-email-yishaih@mellanox.com> X-Mailer: git-send-email 1.8.2.3 In-Reply-To: <1533226416-19122-1-git-send-email-yishaih@mellanox.com> References: <1533226416-19122-1-git-send-email-yishaih@mellanox.com> Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Jason Gunthorpe To access the kabi structs for a WRITE command everything should use the enum name now, not the '_prefix' as these command did. This guarantees that all types match the enum number. Signed-off-by: Jason Gunthorpe Signed-off-by: Yishai Hadas --- libibverbs/cmd_cq.c | 16 ++--- libibverbs/cmd_fallback.c | 54 ++++++++------ libibverbs/cmd_write.h | 176 +++++++++++++++++++++------------------------- 3 files changed, 120 insertions(+), 126 deletions(-) diff --git a/libibverbs/cmd_cq.c b/libibverbs/cmd_cq.c index 3f1254a..0f4780a 100644 --- a/libibverbs/cmd_cq.c +++ b/libibverbs/cmd_cq.c @@ -60,7 +60,7 @@ static int ibv_icmd_create_cq(struct ibv_context *context, int cqe, switch (execute_ioctl_fallback(cq->context, create_cq, cmdb, &ret)) { case TRY_WRITE: { - DECLARE_LEGACY_UHW_BUFS(link, create_cq); + DECLARE_LEGACY_UHW_BUFS(link, IB_USER_VERBS_CMD_CREATE_CQ); *req = (struct ib_uverbs_create_cq){ .user_handle = (uintptr_t)cq, @@ -69,8 +69,7 @@ static int ibv_icmd_create_cq(struct ibv_context *context, int cqe, .comp_channel = channel ? channel->fd : -1, }; - ret = execute_write_bufs(IB_USER_VERBS_CMD_CREATE_CQ, - cq->context, req, resp); + ret = execute_write_bufs(cq->context, req, resp); if (ret) return ret; @@ -80,7 +79,8 @@ static int ibv_icmd_create_cq(struct ibv_context *context, int cqe, return 0; } case TRY_WRITE_EX: { - DECLARE_LEGACY_UHW_BUFS_EX(link, create_cq); + DECLARE_LEGACY_UHW_BUFS_EX(link, + IB_USER_VERBS_EX_CMD_CREATE_CQ); *req = (struct ib_uverbs_ex_create_cq){ .user_handle = (uintptr_t)cq, @@ -90,8 +90,7 @@ static int ibv_icmd_create_cq(struct ibv_context *context, int cqe, .flags = flags, }; - ret = execute_write_bufs_ex(IB_USER_VERBS_EX_CMD_CREATE_CQ, - cq->context, req, resp); + ret = execute_write_bufs_ex(cq->context, req); if (ret) return ret; @@ -152,7 +151,7 @@ int ibv_cmd_destroy_cq(struct ibv_cq *cq) { DECLARE_FBCMD_BUFFER(cmdb, UVERBS_OBJECT_CQ, UVERBS_METHOD_CQ_DESTROY, 2, NULL); - DECLARE_LEGACY_CORE_BUFS(destroy_cq); + DECLARE_LEGACY_CORE_BUFS(IB_USER_VERBS_CMD_DESTROY_CQ); int ret; fill_attr_out_ptr(cmdb, UVERBS_ATTR_DESTROY_CQ_RESP, &resp); @@ -164,8 +163,7 @@ int ibv_cmd_destroy_cq(struct ibv_cq *cq) .cq_handle = cq->handle, }; - ret = execute_write(IB_USER_VERBS_CMD_DESTROY_CQ, cq->context, - req, &resp); + ret = execute_write(cq->context, req, &resp); break; } diff --git a/libibverbs/cmd_fallback.c b/libibverbs/cmd_fallback.c index a943215..2aac6cb 100644 --- a/libibverbs/cmd_fallback.c +++ b/libibverbs/cmd_fallback.c @@ -118,8 +118,9 @@ enum write_fallback _execute_ioctl_fallback(struct ibv_context *ctx, * on the stack (if the driver didn't provide a UHW) or arranged to be * directly before the UHW memory (see _write_set_uhw) */ -void *_write_get_req(struct ibv_command_buffer *link, void *onstack, - size_t size) +void *_write_get_req(struct ibv_command_buffer *link, + struct ib_uverbs_cmd_hdr *onstack, size_t size, + uint32_t cmdnum) { struct ib_uverbs_cmd_hdr *hdr; @@ -137,13 +138,15 @@ void *_write_get_req(struct ibv_command_buffer *link, void *onstack, hdr->in_words = __check_divide(size, 4); } + hdr->command = cmdnum; + return hdr + 1; } -void *_write_get_req_ex(struct ibv_command_buffer *link, void *onstack, - size_t size) +void *_write_get_req_ex(struct ibv_command_buffer *link, struct ex_hdr *onstack, + size_t size, uint32_t cmdnum) { - struct _ib_ex_hdr *hdr; + struct ex_hdr *hdr; size_t full_size = size + sizeof(*hdr); if (link->uhw_in_idx != _UHW_NO_INDEX) { @@ -160,6 +163,9 @@ void *_write_get_req_ex(struct ibv_command_buffer *link, void *onstack, hdr->ex_hdr.provider_in_words = 0; } + hdr->hdr.command = IB_USER_VERBS_CMD_FLAG_EXTENDED | cmdnum; + hdr->ex_hdr.cmd_hdr_reserved = 0; + return hdr + 1; } @@ -186,7 +192,7 @@ void *_write_get_resp(struct ibv_command_buffer *link, } void *_write_get_resp_ex(struct ibv_command_buffer *link, - struct _ib_ex_hdr *hdr, void *onstack, + struct ex_hdr *hdr, void *onstack, size_t resp_size) { void *resp_start; @@ -206,51 +212,59 @@ void *_write_get_resp_ex(struct ibv_command_buffer *link, hdr->ex_hdr.provider_out_words = 0; } + hdr->ex_hdr.response = ioctl_ptr_to_u64(resp_start); + return resp_start; } -int _execute_write_raw(unsigned int cmdnum, struct ibv_context *ctx, - struct ib_uverbs_cmd_hdr *hdr, void *resp) +int _execute_write_raw(struct ibv_context *ctx, struct ib_uverbs_cmd_hdr *hdr, + void *resp) { - hdr->command = cmdnum; - /* * Users assumes the stack buffer is zeroed before passing to the * kernel for writing. */ - memset(resp, 0, hdr->out_words * 4); + if (resp) { + /* + * The helper macros prove the response is at offset 0 of the + * request. + */ + uint64_t *response = (uint64_t *)(hdr + 1); + + *response = ioctl_ptr_to_u64(resp); + memset(resp, 0, hdr->out_words * 4); + } if (write(ctx->cmd_fd, hdr, hdr->in_words * 4) != hdr->in_words * 4) return errno; - VALGRIND_MAKE_MEM_DEFINED(resp, hdr->out_words * 4); + if (resp) + VALGRIND_MAKE_MEM_DEFINED(resp, hdr->out_words * 4); return 0; } -int _execute_write_raw_ex(uint32_t cmdnum, struct ibv_context *ctx, - struct _ib_ex_hdr *hdr, void *resp) +int _execute_write_raw_ex(struct ibv_context *ctx, struct ex_hdr *hdr) { size_t write_bytes = sizeof(*hdr) + (hdr->hdr.in_words + hdr->ex_hdr.provider_in_words) * 8; size_t resp_bytes = (hdr->hdr.out_words + hdr->ex_hdr.provider_out_words) * 8; - - hdr->hdr.command = IB_USER_VERBS_CMD_FLAG_EXTENDED | cmdnum; - hdr->ex_hdr.cmd_hdr_reserved = 0; - hdr->ex_hdr.response = ioctl_ptr_to_u64(resp); + void *resp = (void *)(uintptr_t)hdr->ex_hdr.response; /* * Users assumes the stack buffer is zeroed before passing to the * kernel for writing. */ - memset(resp, 0, resp_bytes); + if (resp) + memset(resp, 0, resp_bytes); if (write(ctx->cmd_fd, hdr, write_bytes) != write_bytes) return errno; - VALGRIND_MAKE_MEM_DEFINED(resp, resp_bytes); + if (resp) + VALGRIND_MAKE_MEM_DEFINED(resp, resp_bytes); return 0; } diff --git a/libibverbs/cmd_write.h b/libibverbs/cmd_write.h index cd0f371..ac59525 100644 --- a/libibverbs/cmd_write.h +++ b/libibverbs/cmd_write.h @@ -45,80 +45,49 @@ static inline struct ib_uverbs_cmd_hdr *get_req_hdr(void *req) return ((struct ib_uverbs_cmd_hdr *)req) - 1; } -struct _ib_ex_hdr { - struct ib_uverbs_cmd_hdr hdr; - struct ib_uverbs_ex_cmd_hdr ex_hdr; -}; - -static inline struct _ib_ex_hdr *get_req_hdr_ex(void *req) +static inline struct ex_hdr *get_req_hdr_ex(void *req) { - return ((struct _ib_ex_hdr *)req) - 1; + return ((struct ex_hdr *)req) - 1; } -/* - * When using these new interfaces the kernel UAPI structs 'ib_uverbs_*' are - * used, not the structs from kern-abi.h. The only difference between the two - * is the inclusion of the header in the kern-abi.h struct. This macro creates - * memory on the stack that includes both the header and the struct. - */ -#define DECLARE_LEGACY_REQ_BUF_CORE(_name, _pattern) \ - struct { \ - struct ib_uverbs_cmd_hdr hdr; \ - struct ib_uverbs_##_pattern core_payload; \ - } _name - -#define DECLARE_LEGACY_REQ_BUF_CORE_EX(_name, _pattern) \ - struct { \ - struct ib_uverbs_cmd_hdr hdr; \ - struct ib_uverbs_ex_cmd_hdr ex_hdr; \ - struct ib_uverbs_ex_##_pattern core_payload; \ - } _name - -void *_write_get_req(struct ibv_command_buffer *link, void *onstack, - size_t size); -void *_write_get_req_ex(struct ibv_command_buffer *link, void *onstack, - size_t size); +void *_write_get_req(struct ibv_command_buffer *link, + struct ib_uverbs_cmd_hdr *onstack, size_t size, + uint32_t cmdnum); +void *_write_get_req_ex(struct ibv_command_buffer *link, struct ex_hdr *onstack, + size_t size, uint32_t cmdnum); void *_write_get_resp(struct ibv_command_buffer *link, struct ib_uverbs_cmd_hdr *hdr, void *onstack, size_t resp_size); void *_write_get_resp_ex(struct ibv_command_buffer *link, - struct _ib_ex_hdr *hdr, void *onstack, + struct ex_hdr *hdr, void *onstack, size_t resp_size); -#define DECLARE_LEGACY_REQ_BUF(_name, _link, _pattern) \ - DECLARE_LEGACY_REQ_BUF_CORE(__##_name##_onstack, _pattern); \ - struct ib_uverbs_##_pattern *_name = \ - _write_get_req(_link, &__##_name##_onstack, sizeof(*_name)) - -#define DECLARE_LEGACY_REQ_BUF_EX(_name, _link, _pattern) \ - DECLARE_LEGACY_REQ_BUF_CORE_EX(__##_name##_onstack, _pattern); \ - struct ib_uverbs_ex_##_pattern *_name = \ - _write_get_req_ex(_link, &__##_name##_onstack, sizeof(*_name)) - -#define DECLARE_LEGACY_RESP_BUF(_name, _link, _req, _pattern) \ - struct ib_uverbs_##_pattern##_resp __##_name##_onstack, \ - *_name = _write_get_resp(_link, get_req_hdr(_req), \ - &__##_name##_onstack, sizeof(*_name)) - -#define DECLARE_LEGACY_RESP_BUF_EX(_name, _link, _req, _pattern) \ - struct ib_uverbs_ex_##_pattern##_resp __##_name##_onstack, \ - *_name = _write_get_resp_ex(_link, get_req_hdr_ex(_req), \ - &__##_name##_onstack, \ - sizeof(*_name)) - /* * This macro creates 'req' and 'resp' pointers in the local stack frame that * point to the core code write command structures patterned off _pattern. * * This should be done before calling execute_write_bufs */ -#define DECLARE_LEGACY_UHW_BUFS(_link, _pattern) \ - DECLARE_LEGACY_REQ_BUF(req, _link, _pattern); \ - DECLARE_LEGACY_RESP_BUF(resp, _link, req, _pattern) +#define DECLARE_LEGACY_UHW_BUFS(_link, _enum) \ + IBV_ABI_REQ(_enum) __req_onstack; \ + IBV_KABI_RESP(_enum) __resp_onstack; \ + static_assert(offsetof(IBV_KABI_REQ(_enum), response) == 0, \ + "Bad response offset"); \ + IBV_KABI_REQ(_enum) *req = _write_get_req(_link, &__req_onstack.hdr, \ + sizeof(*req), _enum); \ + IBV_KABI_RESP(_enum) *resp = ({ \ + void *_resp = _write_get_resp(_link, get_req_hdr(req), \ + &__resp_onstack, sizeof(*resp)); \ + _resp; \ + }) -#define DECLARE_LEGACY_UHW_BUFS_EX(_link, _pattern) \ - DECLARE_LEGACY_REQ_BUF_EX(req, _link, _pattern); \ - DECLARE_LEGACY_RESP_BUF_EX(resp, _link, req, _pattern) +#define DECLARE_LEGACY_UHW_BUFS_EX(_link, _enum) \ + IBV_ABI_REQ(_enum) __req_onstack; \ + IBV_KABI_RESP(_enum) __resp_onstack; \ + IBV_KABI_REQ(_enum) *req = _write_get_req_ex( \ + _link, &__req_onstack.hdr, sizeof(*req), _enum); \ + IBV_KABI_RESP(_enum) *resp = _write_get_resp_ex( \ + _link, get_req_hdr_ex(req), &__resp_onstack, sizeof(*resp)) /* * This macro is used to implement the compatibility command call wrappers. @@ -174,51 +143,68 @@ enum write_fallback _execute_ioctl_fallback(struct ibv_context *ctx, _execute_ioctl_fallback(ctx, _CMD_BIT(cmd_name), cmdb, ret) /* These helpers replace the raw write() and IBV_INIT_CMD macros */ -int _execute_write_raw(unsigned int cmdnum, struct ibv_context *ctx, - struct ib_uverbs_cmd_hdr *req, void *resp); +int _execute_write_raw(struct ibv_context *ctx, struct ib_uverbs_cmd_hdr *req, + void *resp); /* For users of DECLARE_LEGACY_UHW_BUFS */ -#define execute_write_bufs(cmdnum, ctx, req, resp) \ - ({ \ - (req)->response = ioctl_ptr_to_u64(resp); \ - _execute_write_raw(cmdnum, ctx, get_req_hdr(req), resp); \ - }) +#define execute_write_bufs(ctx, req, resp) \ + _execute_write_raw(ctx, get_req_hdr(req), resp) -int _execute_write_raw_ex(uint32_t cmdnum, struct ibv_context *ctx, - struct _ib_ex_hdr *req, void *resp); +int _execute_write_raw_ex(struct ibv_context *ctx, struct ex_hdr *req); /* For users of DECLARE_LEGACY_UHW_BUFS_EX */ -#define execute_write_bufs_ex(cmdnum, ctx, req, resp) \ - _execute_write_raw_ex(cmdnum, ctx, get_req_hdr_ex(req), resp) - -static inline int _execute_write(uint32_t cmdnum, struct ibv_context *ctx, - void *req, size_t req_len, void *resp, - size_t resp_len) -{ - struct ib_uverbs_cmd_hdr *hdr = get_req_hdr(req); - - hdr->in_words = req_len / 4; - hdr->out_words = resp_len / 4; - return _execute_write_raw(cmdnum, ctx, hdr, resp); -} +#define execute_write_bufs_ex(ctx, req) \ + _execute_write_raw_ex(ctx, get_req_hdr_ex(req)) /* For users with no possible UHW bufs. */ -#define DECLARE_LEGACY_CORE_BUFS(_pattern) \ - DECLARE_LEGACY_REQ_BUF_CORE(__req_onstack, _pattern); \ - struct ib_uverbs_##_pattern *const req = &__req_onstack.core_payload; \ - struct ib_uverbs_##_pattern##_resp resp +#define DECLARE_LEGACY_CORE_BUFS(_enum) \ + IBV_ABI_REQ(_enum) __req_onstack; \ + IBV_KABI_RESP(_enum) resp; \ + static_assert(offsetof(IBV_KABI_REQ(_enum), response) == 0, \ + "Bad response offset"); \ + IBV_KABI_REQ(_enum) *const req = ({ \ + __req_onstack.hdr.command = _enum; \ + __req_onstack.hdr.in_words = sizeof(__req_onstack) / 4; \ + __req_onstack.hdr.out_words = sizeof(resp) / 4; \ + &__req_onstack.core_payload; \ + }) +#define DECLARE_LEGACY_CORE_REQ(_enum) \ + IBV_ABI_REQ(_enum) __req_onstack; \ + static_assert(sizeof(IBV_KABI_RESP(_enum)) == 0, \ + "Method has a response!"); \ + IBV_KABI_REQ(_enum) *const req = ({ \ + __req_onstack.hdr.command = _enum; \ + __req_onstack.hdr.in_words = sizeof(__req_onstack) / 4; \ + __req_onstack.hdr.out_words = 0; \ + &__req_onstack.core_payload; \ + }) + +#define DECLARE_LEGACY_CORE_BUFS_EX(_enum) \ + IBV_ABI_REQ(_enum) __req_onstack; \ + IBV_KABI_RESP(_enum) resp; \ + IBV_KABI_REQ(_enum) *const req = ({ \ + __req_onstack.hdr.hdr.command = \ + IB_USER_VERBS_CMD_FLAG_EXTENDED | _enum; \ + __req_onstack.hdr.hdr.in_words = \ + (sizeof(__req_onstack) - sizeof(struct ex_hdr)) / 8; \ + __req_onstack.hdr.hdr.out_words = sizeof(resp) / 8; \ + __req_onstack.hdr.ex_hdr.cmd_hdr_reserved = 0; \ + __req_onstack.hdr.ex_hdr.provider_in_words = 0; \ + __req_onstack.hdr.ex_hdr.provider_out_words = 0; \ + __req_onstack.hdr.ex_hdr.response = \ + (sizeof(resp) == 0) ? 0 : ioctl_ptr_to_u64(&resp); \ + &__req_onstack.core_payload; \ + }) /* * For users with no UHW bufs. To be used in conjunction with * DECLARE_LEGACY_CORE_BUFS. req points to the core payload (with headroom for * the header). */ -#define execute_write(cmdnum, ctx, req, resp) \ - ({ \ - (req)->response = ioctl_ptr_to_u64(resp); \ - _execute_write(cmdnum, ctx, req, sizeof(*req), resp, \ - sizeof(*resp)); \ - }) +#define execute_write(ctx, req, resp) \ + _execute_write_raw(ctx, get_req_hdr(req), resp) +#define execute_write_ex(ctx, req) \ + _execute_write_raw_ex(ctx, get_req_hdr_ex(req)) /* * These two macros are used only with execute_ioctl_fallback - they allow the @@ -247,24 +233,20 @@ _execute_ioctl_only(struct ibv_context *context, struct ibv_command_buffer *cmd, _execute_ioctl_only(ctx, cmdb, ret) #undef execute_write_bufs -static inline int execute_write_bufs(uint32_t cmdnum, - struct ibv_context *ctx, void *req, +static inline int execute_write_bufs(struct ibv_context *ctx, void *req, void *resp) { return ENOSYS; } #undef execute_write_bufs_ex -static inline int execute_write_bufs_ex(uint32_t cmdnum, - struct ibv_context *ctx, void *req, - void *resp) +static inline int execute_write_bufs_ex(struct ibv_context *ctx, void *req) { return ENOSYS; } #undef execute_write -static inline int execute_write(uint32_t cmdnum, - struct ibv_context *ctx, void *req, +static inline int execute_write(struct ibv_context *ctx, void *req, void *resp) { return ENOSYS;