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

["Margolin, Michael" <mrgolin@amazon.com>]

On 12/11/2023 1:26 PM, 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 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

0xffff can't practically be a valid PCI id in this context, but anyway
changed to use validity fields through all the code to be clear on that
andto be more consistent. Sending v3 patch.

Thanks