diff mbox series

[for-next,v2,3/4] RDMA/efa: User/kernel compatibility handshake mechanism

Message ID 20200720080113.13055-4-galpress@amazon.com (mailing list archive)
State Superseded
Headers show
Series Add support for 0xefa1 device | expand

Commit Message

Gal Pressman July 20, 2020, 8:01 a.m. UTC 
Introduce a mechanism that performs an handshake between the userspace
provider and kernel driver which verifies that the user supports all
required features in order to operate correctly.

The handshake verifies the needed functionality by comparing the
reported device caps and the provider caps. If the device reports a
non-zero capability the appropriate comp mask is required from the
userspace provider in order to allocate the context.

Reviewed-by: Shadi Ammouri <sammouri@amazon.com>
Reviewed-by: Yossi Leybovich <sleybo@amazon.com>
Signed-off-by: Gal Pressman <galpress@amazon.com>
---
 drivers/infiniband/hw/efa/efa_verbs.c | 60 +++++++++++++++++++++++++++
 include/uapi/rdma/efa-abi.h           | 10 +++++
 2 files changed, 70 insertions(+)

Comments

kernel test robot July 20, 2020, 5:08 p.m. UTC | #1 
Hi Gal,

I love your patch! Yet something to improve:

[auto build test ERROR on 5f0b2a6093a4d9aab093964c65083fe801ef1e58]

url:    https://github.com/0day-ci/linux/commits/Gal-Pressman/Add-support-for-0xefa1-device/20200720-160419
base:    5f0b2a6093a4d9aab093964c65083fe801ef1e58
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cf1105069648446d58adfb7a6cc590013d6886ba)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

>> drivers/infiniband/hw/efa/efa_verbs.c:1533:3: error: function definition is not allowed here
                   DEFINE_COMP_HANDSHAKE(max_tx_batch, EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH),
                   ^
   drivers/infiniband/hw/efa/efa_verbs.c:1520:4: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                           DEFINE_GET_DEV_ATTR_FUNC(_attr)                        \
                           ^
   drivers/infiniband/hw/efa/efa_verbs.c:1506:2: note: expanded from macro 'DEFINE_GET_DEV_ATTR_FUNC'
           {                                                                      \
           ^
>> drivers/infiniband/hw/efa/efa_verbs.c:1533:3: error: function definition is not allowed here
   drivers/infiniband/hw/efa/efa_verbs.c:1521:4: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                           DEFINE_CHECK_COMP_FUNC(_attr, _mask)                   \
                           ^
   drivers/infiniband/hw/efa/efa_verbs.c:1512:2: note: expanded from macro 'DEFINE_CHECK_COMP_FUNC'
           {                                                                      \
           ^
>> drivers/infiniband/hw/efa/efa_verbs.c:1533:3: error: use of undeclared identifier 'check_comp_max_tx_batch'
   drivers/infiniband/hw/efa/efa_verbs.c:1522:4: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                           check_comp_##_attr;                                    \
                           ^
   <scratch space>:191:1: note: expanded from here
   check_comp_max_tx_batch
   ^
   drivers/infiniband/hw/efa/efa_verbs.c:1534:3: error: function definition is not allowed here
                   DEFINE_COMP_HANDSHAKE(min_sq_depth, EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR),
                   ^
   drivers/infiniband/hw/efa/efa_verbs.c:1520:4: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                           DEFINE_GET_DEV_ATTR_FUNC(_attr)                        \
                           ^
   drivers/infiniband/hw/efa/efa_verbs.c:1506:2: note: expanded from macro 'DEFINE_GET_DEV_ATTR_FUNC'
           {                                                                      \
           ^
   drivers/infiniband/hw/efa/efa_verbs.c:1534:3: error: function definition is not allowed here
   drivers/infiniband/hw/efa/efa_verbs.c:1521:4: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                           DEFINE_CHECK_COMP_FUNC(_attr, _mask)                   \
                           ^
   drivers/infiniband/hw/efa/efa_verbs.c:1512:2: note: expanded from macro 'DEFINE_CHECK_COMP_FUNC'
           {                                                                      \
           ^
>> drivers/infiniband/hw/efa/efa_verbs.c:1534:3: error: use of undeclared identifier 'check_comp_min_sq_depth'
   drivers/infiniband/hw/efa/efa_verbs.c:1522:4: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                           check_comp_##_attr;                                    \
                           ^
   <scratch space>:196:1: note: expanded from here
   check_comp_min_sq_depth
   ^
>> drivers/infiniband/hw/efa/efa_verbs.c:1533:3: error: initializing 'bool (*)(const struct efa_dev *, u32)' (aka '_Bool (*)(const struct efa_dev *, unsigned int)') with an expression of incompatible type 'void'
                   DEFINE_COMP_HANDSHAKE(max_tx_batch, EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH),
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/hw/efa/efa_verbs.c:1519:17: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                   .check_comp = ({                                               \
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/hw/efa/efa_verbs.c:1534:3: error: initializing 'bool (*)(const struct efa_dev *, u32)' (aka '_Bool (*)(const struct efa_dev *, unsigned int)') with an expression of incompatible type 'void'
                   DEFINE_COMP_HANDSHAKE(min_sq_depth, EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR),
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/infiniband/hw/efa/efa_verbs.c:1519:17: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                   .check_comp = ({                                               \
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/infiniband/hw/efa/efa_verbs.c:1539:18: error: invalid application of 'sizeof' to an incomplete type 'struct (anonymous struct at drivers/infiniband/hw/efa/efa_verbs.c:1529:2) []'
           for (i = 0; i < ARRAY_SIZE(user_comp_handshakes); i++) {
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:47:32: note: expanded from macro 'ARRAY_SIZE'
   #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
                                  ^~~~~
>> drivers/infiniband/hw/efa/efa_verbs.c:1526:5: warning: no previous prototype for function 'efa_user_comp_handshake' [-Wmissing-prototypes]
   int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
       ^
   drivers/infiniband/hw/efa/efa_verbs.c:1526:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
   ^
   static 
   1 warning and 9 errors generated.

vim +1533 drivers/infiniband/hw/efa/efa_verbs.c

  1503	
  1504	#define DEFINE_GET_DEV_ATTR_FUNC(_attr)                                        \
  1505		bool dev_attr_##_attr(const struct efa_dev *dev)                       \
  1506		{                                                                      \
  1507			return dev->dev_attr._attr;                                    \
  1508		}
  1509	
  1510	#define DEFINE_CHECK_COMP_FUNC(_attr, _mask)                                   \
  1511		bool check_comp_##_attr(const struct efa_dev *dev, u32 comp_mask)      \
  1512		{                                                                      \
  1513			return !dev_attr_##_attr(dev) || (comp_mask & (_mask));        \
  1514		}
  1515	
  1516	#define DEFINE_COMP_HANDSHAKE(_attr, _mask)                                    \
  1517		{                                                                      \
  1518			.attr = #_attr,                                                \
  1519			.check_comp = ({                                               \
  1520				DEFINE_GET_DEV_ATTR_FUNC(_attr)                        \
  1521				DEFINE_CHECK_COMP_FUNC(_attr, _mask)                   \
  1522				check_comp_##_attr;                                    \
  1523			}),                                                            \
  1524		}
  1525	
> 1526	int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
  1527				    const struct efa_ibv_alloc_ucontext_cmd *cmd)
  1528	{
  1529		struct {
  1530			char *attr;
  1531			bool (*check_comp)(const struct efa_dev *dev, u32 comp_mask);
  1532		} user_comp_handshakes[] = {
> 1533			DEFINE_COMP_HANDSHAKE(max_tx_batch, EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH),
> 1534			DEFINE_COMP_HANDSHAKE(min_sq_depth, EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR),
  1535		};
  1536		struct efa_dev *dev = to_edev(ibucontext->device);
  1537		int i;
  1538	
> 1539		for (i = 0; i < ARRAY_SIZE(user_comp_handshakes); i++) {
  1540			if (!user_comp_handshakes[i].check_comp(dev, cmd->comp_mask)) {
  1541				ibdev_dbg(&dev->ibdev,
  1542					  "Userspace handshake failed for %s attribute\n",
  1543					  user_comp_handshakes[i].attr);
  1544				return -EOPNOTSUPP;
  1545			}
  1546		}
  1547	
  1548		return 0;
  1549	}
  1550	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Gal Pressman July 21, 2020, 11:26 a.m. UTC | #2 
On 20/07/2020 20:08, kernel test robot wrote:
> Hi Gal,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on 5f0b2a6093a4d9aab093964c65083fe801ef1e58]
> 
> url:    https://github.com/0day-ci/linux/commits/Gal-Pressman/Add-support-for-0xefa1-device/20200720-160419
> base:    5f0b2a6093a4d9aab093964c65083fe801ef1e58
> config: x86_64-allyesconfig (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cf1105069648446d58adfb7a6cc590013d6886ba)

Uh, looks like I use some gcc specific stuff here.. I guess it's time to start
checking clang compilation as well :).

Will fix and resubmit.
Nick Desaulniers July 21, 2020, 5:10 p.m. UTC | #3 
On Tue, Jul 21, 2020 at 4:27 AM 'Gal Pressman' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
>
> On 20/07/2020 20:08, kernel test robot wrote:
> > Hi Gal,
> >
> > I love your patch! Yet something to improve:
> >
> > [auto build test ERROR on 5f0b2a6093a4d9aab093964c65083fe801ef1e58]
> >
> > url:    https://github.com/0day-ci/linux/commits/Gal-Pressman/Add-support-for-0xefa1-device/20200720-160419
> > base:    5f0b2a6093a4d9aab093964c65083fe801ef1e58
> > config: x86_64-allyesconfig (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cf1105069648446d58adfb7a6cc590013d6886ba)
>
> Uh, looks like I use some gcc specific stuff here.. I guess it's time to start
> checking clang compilation as well :).
>
> Will fix and resubmit.

>> drivers/infiniband/hw/efa/efa_verbs.c:1539:18: error: invalid application of 'sizeof' to an incomplete type 'struct (anonymous struct at drivers/infiniband/hw/efa/efa_verbs.c:1529:2) []'
           for (i = 0; i < ARRAY_SIZE(user_comp_handshakes); i++) {
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

is user_comp_handshakes forward declared but not defined for an allyesconfig?
Gal Pressman July 22, 2020, 6:35 a.m. UTC | #4 
On 21/07/2020 20:10, Nick Desaulniers wrote:
> On Tue, Jul 21, 2020 at 4:27 AM 'Gal Pressman' via Clang Built Linux
> <clang-built-linux@googlegroups.com> wrote:
>>
>> On 20/07/2020 20:08, kernel test robot wrote:
>>> Hi Gal,
>>>
>>> I love your patch! Yet something to improve:
>>>
>>> [auto build test ERROR on 5f0b2a6093a4d9aab093964c65083fe801ef1e58]
>>>
>>> url:    https://github.com/0day-ci/linux/commits/Gal-Pressman/Add-support-for-0xefa1-device/20200720-160419
>>> base:    5f0b2a6093a4d9aab093964c65083fe801ef1e58
>>> config: x86_64-allyesconfig (attached as .config)
>>> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cf1105069648446d58adfb7a6cc590013d6886ba)
>>
>> Uh, looks like I use some gcc specific stuff here.. I guess it's time to start
>> checking clang compilation as well :).
>>
>> Will fix and resubmit.
> 
>>> drivers/infiniband/hw/efa/efa_verbs.c:1539:18: error: invalid application of 'sizeof' to an incomplete type 'struct (anonymous struct at drivers/infiniband/hw/efa/efa_verbs.c:1529:2) []'
>            for (i = 0; i < ARRAY_SIZE(user_comp_handshakes); i++) {
>                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> is user_comp_handshakes forward declared but not defined for an allyesconfig?
> 

I don't think that's the issue here, the real problem is the first error:

>> drivers/infiniband/hw/efa/efa_verbs.c:1533:3: error: function definition is not allowed here
                   DEFINE_COMP_HANDSHAKE(max_tx_batch, EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH),
                   ^
   drivers/infiniband/hw/efa/efa_verbs.c:1520:4: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
                           DEFINE_GET_DEV_ATTR_FUNC(_attr)                        \
                           ^
   drivers/infiniband/hw/efa/efa_verbs.c:1506:2: note: expanded from macro 'DEFINE_GET_DEV_ATTR_FUNC'


Apparently the braced group (is that how its called?) is supported by gcc, but not clang.
Nathan Chancellor July 22, 2020, 3:51 p.m. UTC | #5 
On Wed, Jul 22, 2020 at 09:35:14AM +0300, 'Gal Pressman' via Clang Built Linux wrote:
> On 21/07/2020 20:10, Nick Desaulniers wrote:
> > On Tue, Jul 21, 2020 at 4:27 AM 'Gal Pressman' via Clang Built Linux
> > <clang-built-linux@googlegroups.com> wrote:
> >>
> >> On 20/07/2020 20:08, kernel test robot wrote:
> >>> Hi Gal,
> >>>
> >>> I love your patch! Yet something to improve:
> >>>
> >>> [auto build test ERROR on 5f0b2a6093a4d9aab093964c65083fe801ef1e58]
> >>>
> >>> url:    https://github.com/0day-ci/linux/commits/Gal-Pressman/Add-support-for-0xefa1-device/20200720-160419
> >>> base:    5f0b2a6093a4d9aab093964c65083fe801ef1e58
> >>> config: x86_64-allyesconfig (attached as .config)
> >>> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cf1105069648446d58adfb7a6cc590013d6886ba)
> >>
> >> Uh, looks like I use some gcc specific stuff here.. I guess it's time to start
> >> checking clang compilation as well :).
> >>
> >> Will fix and resubmit.
> > 
> >>> drivers/infiniband/hw/efa/efa_verbs.c:1539:18: error: invalid application of 'sizeof' to an incomplete type 'struct (anonymous struct at drivers/infiniband/hw/efa/efa_verbs.c:1529:2) []'
> >            for (i = 0; i < ARRAY_SIZE(user_comp_handshakes); i++) {
> >                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > is user_comp_handshakes forward declared but not defined for an allyesconfig?
> > 
> 
> I don't think that's the issue here, the real problem is the first error:
> 
> >> drivers/infiniband/hw/efa/efa_verbs.c:1533:3: error: function definition is not allowed here
>                    DEFINE_COMP_HANDSHAKE(max_tx_batch, EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH),
>                    ^
>    drivers/infiniband/hw/efa/efa_verbs.c:1520:4: note: expanded from macro 'DEFINE_COMP_HANDSHAKE'
>                            DEFINE_GET_DEV_ATTR_FUNC(_attr)                        \
>                            ^
>    drivers/infiniband/hw/efa/efa_verbs.c:1506:2: note: expanded from macro 'DEFINE_GET_DEV_ATTR_FUNC'
> 
> 
> Apparently the braced group (is that how its called?) is supported by gcc, but not clang.
> 

I believe you are trying to use nested functions, which are not
supported by clang:

https://gcc.gnu.org/onlinedocs/gcc/Nested-Functions.html

clang supports both scoped statements and GNU C statement expressions
but you are trying to define a function within a GNU C statement
expression and use it in a designated initializer (in
DEFINE_COMP_HANDSHAKE with DEFINE_GET_DEV_ATTR_FUNC and
DEFINE_CHECK_COMP_FUNC), which causes a problem with clang.

Cheers,
Nathan
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
index 26102ab333b2..f60bf9ce656f 100644
--- a/drivers/infiniband/hw/efa/efa_verbs.c
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -1501,11 +1501,59 @@  static int efa_dealloc_uar(struct efa_dev *dev, u16 uarn)
 	return efa_com_dealloc_uar(&dev->edev, &params);
 }
 
+#define DEFINE_GET_DEV_ATTR_FUNC(_attr)                                        \
+	bool dev_attr_##_attr(const struct efa_dev *dev)                       \
+	{                                                                      \
+		return dev->dev_attr._attr;                                    \
+	}
+
+#define DEFINE_CHECK_COMP_FUNC(_attr, _mask)                                   \
+	bool check_comp_##_attr(const struct efa_dev *dev, u32 comp_mask)      \
+	{                                                                      \
+		return !dev_attr_##_attr(dev) || (comp_mask & (_mask));        \
+	}
+
+#define DEFINE_COMP_HANDSHAKE(_attr, _mask)                                    \
+	{                                                                      \
+		.attr = #_attr,                                                \
+		.check_comp = ({                                               \
+			DEFINE_GET_DEV_ATTR_FUNC(_attr)                        \
+			DEFINE_CHECK_COMP_FUNC(_attr, _mask)                   \
+			check_comp_##_attr;                                    \
+		}),                                                            \
+	}
+
+int efa_user_comp_handshake(const struct ib_ucontext *ibucontext,
+			    const struct efa_ibv_alloc_ucontext_cmd *cmd)
+{
+	struct {
+		char *attr;
+		bool (*check_comp)(const struct efa_dev *dev, u32 comp_mask);
+	} user_comp_handshakes[] = {
+		DEFINE_COMP_HANDSHAKE(max_tx_batch, EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH),
+		DEFINE_COMP_HANDSHAKE(min_sq_depth, EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR),
+	};
+	struct efa_dev *dev = to_edev(ibucontext->device);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(user_comp_handshakes); i++) {
+		if (!user_comp_handshakes[i].check_comp(dev, cmd->comp_mask)) {
+			ibdev_dbg(&dev->ibdev,
+				  "Userspace handshake failed for %s attribute\n",
+				  user_comp_handshakes[i].attr);
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return 0;
+}
+
 int efa_alloc_ucontext(struct ib_ucontext *ibucontext, struct ib_udata *udata)
 {
 	struct efa_ucontext *ucontext = to_eucontext(ibucontext);
 	struct efa_dev *dev = to_edev(ibucontext->device);
 	struct efa_ibv_alloc_ucontext_resp resp = {};
+	struct efa_ibv_alloc_ucontext_cmd cmd = {};
 	struct efa_com_alloc_uar_result result;
 	int err;
 
@@ -1514,6 +1562,18 @@  int efa_alloc_ucontext(struct ib_ucontext *ibucontext, struct ib_udata *udata)
 	 * we will ack input fields in our response.
 	 */
 
+	err = ib_copy_from_udata(&cmd, udata,
+				 min(sizeof(cmd), udata->inlen));
+	if (err) {
+		ibdev_dbg(&dev->ibdev,
+			  "Cannot copy udata for alloc_ucontext\n");
+		goto err_out;
+	}
+
+	err = efa_user_comp_handshake(ibucontext, &cmd);
+	if (err)
+		goto err_out;
+
 	err = efa_com_alloc_uar(&dev->edev, &result);
 	if (err)
 		goto err_out;
diff --git a/include/uapi/rdma/efa-abi.h b/include/uapi/rdma/efa-abi.h
index 7ef2306f8dd4..507a2862bedb 100644
--- a/include/uapi/rdma/efa-abi.h
+++ b/include/uapi/rdma/efa-abi.h
@@ -20,6 +20,16 @@ 
  * hex bit offset of the field.
  */
 
+enum {
+	EFA_ALLOC_UCONTEXT_CMD_COMP_TX_BATCH  = 1 << 0,
+	EFA_ALLOC_UCONTEXT_CMD_COMP_MIN_SQ_WR = 1 << 1,
+};
+
+struct efa_ibv_alloc_ucontext_cmd {
+	__u32 comp_mask;
+	__u8 reserved_20[4];
+};
+
 enum efa_ibv_user_cmds_supp_udata {
 	EFA_USER_CMDS_SUPP_UDATA_QUERY_DEVICE = 1 << 0,
 	EFA_USER_CMDS_SUPP_UDATA_CREATE_AH    = 1 << 1,