Re: [PATCH AUTOSEL 6.18-6.12] RDMA/efa: Add EFA 0xefa4 PCI ID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] RDMA/efa: broken >=4GB MR registration on 0xefa4 in efa_register_mr()
- [High] RDMA/efa: missing AH cache in efa_com_create_ah() for 0xefa4 devices
--

commit b286b983be1758d745ca136c6fbfaf1703d09edf
Author: Anas Mousa <anasmous@amazon.com>

RDMA/efa: Add EFA 0xefa4 PCI ID

This commit adds the PCI device ID 0xefa4 (PCI_DEV_ID_EFA4_VF) to
efa_pci_tbl so the EFA driver can bind to 0xefa4 devices.

Link: https://patch.msgid.link/20260712134413.19226-3-mrgolin@amazon.com

> diff --git a/drivers/infiniband/hw/efa/efa_main.c b/drivers/infiniband/hw/efa/efa_main.c
> index 6c415b9adb5fe..460d77d965057 100644
> --- a/drivers/infiniband/hw/efa/efa_main.c
> +++ b/drivers/infiniband/hw/efa/efa_main.c
> @@ -17,12 +17,14 @@
>  #define PCI_DEV_ID_EFA1_VF 0xefa1
>  #define PCI_DEV_ID_EFA2_VF 0xefa2
>  #define PCI_DEV_ID_EFA3_VF 0xefa3
> +#define PCI_DEV_ID_EFA4_VF 0xefa4
>  
>  static const struct pci_device_id efa_pci_tbl[] = {
>  	{ PCI_VDEVICE(AMAZON, PCI_DEV_ID_EFA0_VF) },
>  	{ PCI_VDEVICE(AMAZON, PCI_DEV_ID_EFA1_VF) },
>  	{ PCI_VDEVICE(AMAZON, PCI_DEV_ID_EFA2_VF) },
>  	{ PCI_VDEVICE(AMAZON, PCI_DEV_ID_EFA3_VF) },
> +	{ PCI_VDEVICE(AMAZON, PCI_DEV_ID_EFA4_VF) },

[Severity: High]
Does enabling PCI_DEV_ID_EFA4_VF in efa_pci_tbl break memory region
registration when a page size of 4GB or larger (page_shift >= 32) is
selected on 0xefa4 devices?

When 0xefa4 devices advertise >= 4GB page sizes in dev_attr.page_size_cap
and a consumer registers a large memory region via efa_register_mr(), the
64-bit unsigned long return value of ib_umem_find_best_pgsz() is stored in
a 32-bit unsigned int pg_sz:

drivers/infiniband/hw/efa/efa_verbs.c:efa_register_mr() {
	unsigned int pg_sz;
	...
	pg_sz = ib_umem_find_best_pgsz(mr->umem,
				       dev->dev_attr.page_size_cap,
				       virt_addr);
	if (!pg_sz) {
		ibdev_dbg(&dev->ibdev, "Failed to find a suitable page size in page_size_cap %#llx\n",
			  dev->dev_attr.page_size_cap);
		return -EOPNOTSUPP;
	}

	params.page_shift = order_base_2(pg_sz);
}

When ib_umem_find_best_pgsz() selects a power-of-two page size >= 4GB,
assigning it to unsigned int pg_sz truncates the value to 0 and causes
efa_register_mr() to fail with -EOPNOTSUPP.

In addition, upstream commit 6a57af838b1a5 ("RDMA/efa: Add EFA 0xefa4 PCI
ID") was submitted as patch 2/2 alongside commit 4266547bdce74 ("RDMA/efa:
Extend page-shift field in MR registration"), which widens
EFA_ADMIN_REG_MR_CMD_PHYS_PAGE_SIZE_SHIFT_MASK from GENMASK(4, 0) to
GENMASK(5, 0) for >4GB page sizes. Without that companion commit,
drivers/infiniband/hw/efa/efa_admin_cmds_defs.h still defines:

#define EFA_ADMIN_REG_MR_CMD_PHYS_PAGE_SIZE_SHIFT_MASK      GENMASK(4, 0)

So in efa_com_register_mr() in drivers/infiniband/hw/efa/efa_com_cmd.c:

	EFA_SET(&mr_cmd.flags, EFA_ADMIN_REG_MR_CMD_PHYS_PAGE_SIZE_SHIFT,
		params->page_shift);

bit 5 of params->page_shift is masked off for any page_shift >= 32. Should
pg_sz be an unsigned long and commit 4266547bdce74 be included as well?

[Severity: High]
Will creating multiple address handles to the same remote GID on the same
PD fail on 0xefa4 hardware without the address handle cache commits?

Upstream commits 97ba15272e8e ("RDMA/efa: Add initialization of AH cache
rhashtable") and 234895fa8be0 ("RDMA/efa: Add AH cache handling on create
and destroy AH") were merged right before commit 6a57af838b1a5 because new
EFA devices do not support creating multiple address handles to the same
remote on the same PD.

Without those commits backported, calling efa_create_ah() ->
efa_com_create_ah() for the same (pdn, dest_addr) pair unconditionally
sends a new EFA_ADMIN_CREATE_AH command to the device instead of
deduplicating and refcounting via efa_ah_cache_get():

drivers/infiniband/hw/efa/efa_com_cmd.c:efa_com_create_ah() {
	...
	ah_cmd.aq_common_desc.opcode = EFA_ADMIN_CREATE_AH;

	memcpy(ah_cmd.dest_addr, params->dest_addr, sizeof(ah_cmd.dest_addr));
	ah_cmd.pd = params->pdn;

	err = efa_com_cmd_exec(aq,
			       (struct efa_admin_aq_entry *)&ah_cmd,
			       sizeof(ah_cmd),
			       (struct efa_admin_acq_entry *)&cmd_completion,
			       sizeof(cmd_completion));
}

Should commits 97ba15272e8e and 234895fa8be0 also be backported before
enabling PCI_DEV_ID_EFA4_VF?

>  	{ }
>  };
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-330-sashal@kernel.org?part=1