RDMA/hfi1: Allocate device data after PCI initialization

After the preceding changes, module parameter validation and common PCI
setup no longer need hfi1_devdata. init_one() nevertheless allocates it
first, so failures in those early steps return without releasing it.

Move the allocation after hfi1_pcie_init() and return directly when no
resources are held. Use dev_err() and pci_info() for diagnostics emitted
before allocation so they retain the adapter BDF. Once PCI setup succeeds,
unwind allocation failures through hfi1_pcie_cleanup() to disable the
device and release its regions.

Some PCI error delivery paths are not serialized against probe. Keep their
diagnostics based on pci_dev and skip resume while driver data is absent,
preventing recovery from dereferencing a missing hfi1_devdata.

Fixes: 57f97e9662 ("IB/hfi1: Get the hfi1_devdata structure as early as possible")
Reported-by: Dawei Feng <dawei.feng@seu.edu.cn>
Closes: https://lore.kernel.org/all/20260627060159.2543686-1-dawei.feng@seu.edu.cn/
Link: https://patch.msgid.link/20260708-clean-init-one-hfi1-v1-8-b9e9641268a5@nvidia.com
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
This commit is contained in:
Leon Romanovsky 2026-07-13 07:41:25 -04:00
parent 8e17e101e0
commit 9f674ba674
2 changed files with 29 additions and 33 deletions

View File

@ -1569,25 +1569,16 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
/* First, lock the non-writable module parameters */
HFI1_CAP_LOCK();
/* Allocate the dd so we can get to work */
dd = hfi1_alloc_devdata(pdev, NUM_IB_PORTS *
sizeof(struct hfi1_pportdata));
if (IS_ERR(dd)) {
ret = PTR_ERR(dd);
goto bail;
}
/* Validate some global module parameters */
ret = hfi1_validate_rcvhdrcnt(pdev, rcvhdrcnt);
if (ret)
goto bail;
return ret;
/* use the encoding function as a sanitization check */
if (!encode_rcv_header_entry_size(hfi1_hdrq_entsize)) {
dd_dev_err(dd, "Invalid HdrQ Entry size %u\n",
hfi1_hdrq_entsize);
ret = -EINVAL;
goto bail;
dev_err(&pdev->dev, "Invalid HdrQ Entry size %u\n",
hfi1_hdrq_entsize);
return -EINVAL;
}
/* The receive eager buffer size must be set before the receive
@ -1607,12 +1598,10 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
clamp_val(eager_buffer_size,
MIN_EAGER_BUFFER * 8,
MAX_EAGER_BUFFER_TOTAL);
dd_dev_info(dd, "Eager buffer size %u\n",
eager_buffer_size);
pci_info(pdev, "Eager buffer size %u\n", eager_buffer_size);
} else {
dd_dev_err(dd, "Invalid Eager buffer size of 0\n");
ret = -EINVAL;
goto bail;
dev_err(&pdev->dev, "Invalid Eager buffer size of 0\n");
return -EINVAL;
}
/* restrict value of hfi1_rcvarr_split */
@ -1620,15 +1609,22 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
ret = hfi1_pcie_init(pdev);
if (ret)
goto bail;
return ret;
/* Allocate the dd so we can get to work */
dd = hfi1_alloc_devdata(pdev, NUM_IB_PORTS *
sizeof(struct hfi1_pportdata));
if (IS_ERR(dd)) {
ret = PTR_ERR(dd);
goto clean_pcie;
}
ret = create_workqueues(dd);
if (ret)
goto free_devdata;
/*
* Do device-specific initialization, function table setup, dd
* allocation, etc.
* Do device-specific initialization, function table setup, etc.
*/
ret = hfi1_init_dd(dd);
if (ret)
@ -1679,7 +1675,7 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
postinit_cleanup(dd);
if (initfail)
ret = initfail;
goto bail; /* everything already cleaned */
return ret; /* everything already cleaned */
}
sdma_start(dd);
@ -1690,8 +1686,8 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
destroy_workqueues(dd);
free_devdata:
hfi1_free_devdata(dd);
clean_pcie:
hfi1_pcie_cleanup(pdev);
bail:
return ret;
}

View File

@ -514,29 +514,28 @@ pci_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
switch (state) {
case pci_channel_io_normal:
dd_dev_info(dd, "State Normal, ignoring\n");
dev_info(&pdev->dev, "State Normal, ignoring\n");
break;
case pci_channel_io_frozen:
dd_dev_info(dd, "State Frozen, requesting reset\n");
dev_info(&pdev->dev, "State Frozen, requesting reset\n");
pci_disable_device(pdev);
ret = PCI_ERS_RESULT_NEED_RESET;
break;
case pci_channel_io_perm_failure:
dev_info(&pdev->dev, "State Permanent Failure, disabling\n");
if (dd) {
dd_dev_info(dd, "State Permanent Failure, disabling\n");
/* no more register accesses! */
dd->flags &= ~HFI1_PRESENT;
hfi1_disable_after_error(dd);
}
/* else early, or other problem */
ret = PCI_ERS_RESULT_DISCONNECT;
break;
default: /* shouldn't happen */
dd_dev_info(dd, "HFI1 PCI errors detected (state %d)\n",
state);
dev_info(&pdev->dev, "HFI1 PCI errors detected (state %d)\n",
state);
break;
}
return ret;
@ -563,9 +562,7 @@ pci_mmio_enabled(struct pci_dev *pdev)
static pci_ers_result_t
pci_slot_reset(struct pci_dev *pdev)
{
struct hfi1_devdata *dd = pci_get_drvdata(pdev);
dd_dev_info(dd, "HFI1 slot_reset function called, ignored\n");
dev_info(&pdev->dev, "HFI1 slot_reset function called, ignored\n");
return PCI_ERS_RESULT_CAN_RECOVER;
}
@ -574,7 +571,10 @@ pci_resume(struct pci_dev *pdev)
{
struct hfi1_devdata *dd = pci_get_drvdata(pdev);
dd_dev_info(dd, "HFI1 resume function called\n");
dev_info(&pdev->dev, "HFI1 resume function called\n");
if (!dd)
return;
/*
* Running jobs will fail, since it's asynchronous
* unlike sysfs-requested reset. Better than