On Thu, Jun 10, 2021 at 08:42:24AM -0300, Jason Gunthorpe wrote:
> On Thu, Jun 10, 2021 at 08:50:59AM +0300, Leon Romanovsky wrote:
> > On Wed, Jun 09, 2021 at 06:59:30PM +0300, Yishai Hadas wrote:
> > > From: Patrisious Haddad <phaddad@xxxxxxxxxx>
> > >
> > > Implement the ibv_query_qp_data_in_order() verb by using DEVX to read
> > > from firmware the 'in_order_data' capability.
> > >
> > > Signed-off-by: Patrisious Haddad <phaddad@xxxxxxxxxx>
> > > Reviewed-by: Maor Gottlieb <maorg@xxxxxxxxxx>
> > > Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxx>
> > > providers/mlx5/mlx5.c | 1 +
> > > providers/mlx5/mlx5.h | 3 +++
> > > providers/mlx5/mlx5_ifc.h | 39 +++++++++++++++++++++++++++++++--
> > > providers/mlx5/verbs.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 96 insertions(+), 2 deletions(-)
> >
> > <...>
> >
> > > +int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > > + uint32_t flags)
> > > +{
> > > + uint32_t in_qp[DEVX_ST_SZ_DW(query_qp_in)] = {};
> > > + uint32_t out_qp[DEVX_ST_SZ_DW(query_qp_out)] = {};
> > > + struct mlx5_context *mctx = to_mctx(qp->context);
> > > + struct mlx5_qp *mqp = to_mqp(qp);
> > > + int ret;
> > > +
> > > +/* Currently this API is only supported for x86 architectures since most
> > > + * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > > + */
> > > +#if !defined(__i386__) && !defined(__x86_64__)
> > > + return 0;
> > > +#endif
> >
> > Does it compile without warnings/errors on such platforms?
> > You have "return 0;" in the middle of function, so the right thing to do
> > it is to write with "#if ..." over function or inside like below, as
> > long as "#else" exists.
> >
> > int mlx5_query_qp_data_in_order(struct ibv_qp *qp, enum ibv_wr_opcode op,
> > uint32_t flags)
> > {
> > #if !defined(__i386__) && !defined(__x86_64__)
> > /* Currently this API is only supported for x86 architectures since most
> > * non-x86 platforms are known to be OOO and need to do a per-platform study.
> > */
> > return 0;
> > #else
> > .....
> > #endif
>
> We should probably put the above in the core code anyhow
Agree, it makes sense.
Thanks
>
> Jason
