From 9f674ba674a0d1d6c418752c237364e12620dd89 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Mon, 13 Jul 2026 07:41:25 -0400 Subject: [PATCH] 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: 57f97e96625f ("IB/hfi1: Get the hfi1_devdata structure as early as possible") Reported-by: Dawei Feng 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 --- drivers/infiniband/hw/hfi1/init.c | 42 ++++++++++++++----------------- drivers/infiniband/hw/hfi1/pcie.c | 20 +++++++-------- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c index 79e253ac61f3..0545180b5a11 100644 --- a/drivers/infiniband/hw/hfi1/init.c +++ b/drivers/infiniband/hw/hfi1/init.c @@ -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; } diff --git a/drivers/infiniband/hw/hfi1/pcie.c b/drivers/infiniband/hw/hfi1/pcie.c index 7ca8f07ba43e..1154b8cc713c 100644 --- a/drivers/infiniband/hw/hfi1/pcie.c +++ b/drivers/infiniband/hw/hfi1/pcie.c @@ -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