On Wed, Jul 01, 2020 at 03:28:24PM +0300, Yishai Hadas wrote:
> On 6/23/2020 8:33 PM, Jason Gunthorpe wrote:
> > On Sun, Jun 21, 2020 at 11:44:52AM +0300, Yishai Hadas wrote:
> > > On 6/19/2020 3:50 PM, Jason Gunthorpe wrote:
> > > > On Wed, Jun 17, 2020 at 10:45:53AM +0300, Yishai Hadas wrote:
> > > > > Implement the import/unimport MR verbs based on their definition as
> > > > > described in the man page.
> > > > >
> > > > > It uses the query MR KABI to retrieve the original MR properties based
> > > > > on its given handle.
> > > > >
> > > > > Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxxxx>
> > > > > libibverbs/cmd_mr.c | 35 +++++++++++++++++++++++++++++++++++
> > > > > libibverbs/driver.h | 3 +++
> > > > > libibverbs/libibverbs.map.in | 1 +
> > > > > providers/mlx5/mlx5.c | 2 ++
> > > > > providers/mlx5/mlx5.h | 3 +++
> > > > > providers/mlx5/verbs.c | 24 ++++++++++++++++++++++++
> > > > > 6 files changed, 68 insertions(+)
> > > > >
> > > > > diff --git a/libibverbs/cmd_mr.c b/libibverbs/cmd_mr.c
> > > > > index cb729b6..6984948 100644
> > > > > +++ b/libibverbs/cmd_mr.c
> > > > > @@ -85,3 +85,38 @@ int ibv_cmd_dereg_mr(struct verbs_mr *vmr)
> > > > > return ret;
> > > > > return 0;
> > > > > }
> > > > > +
> > > > > +int ibv_cmd_query_mr(struct ibv_pd *pd, struct verbs_mr *vmr,
> > > > > + uint32_t mr_handle)
> > > > > +{
> > > > > + DECLARE_FBCMD_BUFFER(cmd, UVERBS_OBJECT_MR,
> > > > > + UVERBS_METHOD_QUERY_MR,
> > > > > + 5, NULL);
> > > > > + struct ibv_mr *mr = &vmr->ibv_mr;
> > > > > + uint64_t iova;
> > > > > + int ret;
> > > > > +
> > > > > + fill_attr_in_obj(cmd, UVERBS_ATTR_QUERY_MR_HANDLE, mr_handle);
> > > > > + fill_attr_out_ptr(cmd, UVERBS_ATTR_QUERY_MR_RESP_LKEY,
> > > > > + &mr->lkey);
> > > > > + fill_attr_out_ptr(cmd, UVERBS_ATTR_QUERY_MR_RESP_RKEY,
> > > > > + &mr->rkey);
> > > > > + fill_attr_out_ptr(cmd, UVERBS_ATTR_QUERY_MR_RESP_LENGTH,
> > > > > + &mr->length);
> > > > > + /* The iova may be used down the road, let's have it ready from kernel */
> > > > > + fill_attr_out_ptr(cmd, UVERBS_ATTR_QUERY_MR_RESP_IOVA,
> > > > > + &iova);
> > > >
> > > > There isn't much reason to fill the attribute here..
> > > >
> > >
> > > We have defined this attribute from kernel perspective to be mandatory from
> > > day one as of other attributes above.
> > > Are you suggesting to change in kernel to let this attribute be optional and
> > > not fill it here ? other alternative ?
> >
> > I'm not sure output attributes should be marked as mandatory?
> >
>
> OK, this attribute was changed to be optional in V1 kernel series, the PR
> was updated to not fill this attribute at all, thanks.
The kernel series looks OK, someone will send the v1?
Jason
