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@xxxxxxxxxx> >>>> Reviewed-by: Firas Jahjah <firasj@xxxxxxxxxx> >>>> Signed-off-by: Michael Margolin <mrgolin@xxxxxxxxxx> >>>> --- >>>> 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
