Re: [PATCH for-next v2] RDMA/efa: Add EFA query MR support

[Leon Romanovsky <leon@kernel.org>]

On Mon, Dec 11, 2023 at 12:35:34PM +0200, Margolin, Michael wrote:
> 
> On 12/11/2023 10:10 AM, Leon Romanovsky wrote:
> > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> >
> >
> >
> > On Thu, Dec 07, 2023 at 02:27:48PM +0000, Michael Margolin wrote:
> >> Add EFA driver uapi definitions and register a new query MR method that
> >> currently returns the physical PCI buses' IDs the device is using to
> >> reach the MR. Update admin definitions and efa-abi accordingly.
> >>
> >> Reviewed-by: Anas Mousa <anasmous@amazon.com>
> >> Reviewed-by: Firas Jahjah <firasj@amazon.com>
> >> Signed-off-by: Michael Margolin <mrgolin@amazon.com>
> >> ---
> >>  drivers/infiniband/hw/efa/efa.h               |  5 +-
> >>  .../infiniband/hw/efa/efa_admin_cmds_defs.h   | 31 ++++++++
> >>  drivers/infiniband/hw/efa/efa_com_cmd.c       |  6 ++
> >>  drivers/infiniband/hw/efa/efa_com_cmd.h       |  4 +
> >>  drivers/infiniband/hw/efa/efa_main.c          |  5 ++
> >>  drivers/infiniband/hw/efa/efa_verbs.c         | 77 +++++++++++++++++++
> >>  include/uapi/rdma/efa-abi.h                   | 19 +++++
> >>  7 files changed, 146 insertions(+), 1 deletion(-)
> > <...>
> >
> >> +     /*
> >> +      * Mask indicating which fields have valid values
> >> +      * 0 : recv_pci_bus_id
> >> +      * 1 : rdma_read_pci_bus_id
> >> +      * 2 : rdma_recv_pci_bus_id
> >> +      */
> >> +     u8 validity;
> > <...>
> >
> >>  #define EFA_GID_SIZE 16
> >> +#define EFA_INVALID_PCI_BUS_ID 0xffff
> > Is 0xffff value guaranteed by PCI subsystem to be invalid? Why don't you
> > provide "validity" field to userspace instead?
> 
> The 0xffff value in only used internally in the driver to indicate an
> invalid id and isn't exposed to userspace. For userspace there is a
> validity field as you suggested:
> 
> +       return uverbs_copy_to(attrs,
> EFA_IB_ATTR_QUERY_MR_RESP_PCI_BUS_ID_VALIDITY,
> +                             &pci_bus_id_validity,
> sizeof(pci_bus_id_validity));

So please rely on your EFA_GET(&cmd_completion.validity, EFA_ADMIN_XXX_PCI_BUS_ID)
checks when you fill pci_bus_id_validity and not on 0xffff value which can be
valid from PCI perspective.

Thanks

> 
> 
> Thanks
>