Re: [PATCH net-next] net/mlx5: Add reset_prepare and reset_done callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net/mlx5: concurrent external PCI reset skips driver teardown
--

commit c754f5586b2dc54ce6e7b9bf66e23475b9b184d4
Author: Akiva Goldberger <agoldberger@nvidia.com>

net/mlx5: Add reset_prepare and reset_done callbacks

This commit adds reset_prepare and reset_done callbacks for the mlx5 driver
to quiesce and restore resources around a PCI reset. It also adds a flag to
serialize PCI resets with other flows like FW reset.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 1a45474ce03db..fe7c0331b183b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -2138,10 +2138,97 @@ static void mlx5_pci_resume(struct pci_dev *pdev)
[ ... ]
> +static void mlx5_pci_reset_prepare(struct pci_dev *pdev)
> +{
> +	struct mlx5_core_dev *dev = pci_get_drvdata(pdev);
> +	struct devlink *devlink;
> +	bool fw_reset_in_prog;
> +
> +	if (!dev)
> +		return;
> +
> +	devlink = priv_to_devlink(dev);
> +
> +	devl_lock(devlink);
> +	fw_reset_in_prog = mlx5_fw_reset_in_progress(dev);
> +	if (!fw_reset_in_prog)
> +		dev->pci_reset_in_progress = true;
> +	devl_unlock(devlink);
> +
> +	if (fw_reset_in_prog) {
> +		mlx5_core_info(dev,
> +			       "%s: fw reset in progress, leaving drain to fw_reset flow\n",
> +			       __func__);
> +		return;
> +	}

[Severity: High]
Does this early return assume the caller is the FW reset flow?

If mlx5_sync_reset_request_event() sets MLX5_FW_RESET_FLAGS_RESET_IN_PROGRESS
while waiting for the FW to send a tear-down event, and a concurrent external
PCI reset (e.g., via sysfs) invokes pci_reset_function() during this window,
it will call mlx5_pci_reset_prepare().

Since fw_reset_in_prog would be true, it returns immediately, skipping
mlx5_unload_one().

Can this cause the PCI core to perform a physical reset on the device
while the driver is fully active, allowing active DMA queues to access wiped
hardware and potentially leading to crashes or hardware errors?

> +
> +	mlx5_core_info(dev, "%s: resetting, device state = %d, pci_status = %d, intf_state = 0x%lx\n",
> +		       __func__, dev->state, dev->pci_status, dev->intf_state);
> +
> +	mlx5_enter_error_state(dev, true);
> +	mlx5_drain_health_wq(dev);
> +	mlx5_unload_one(dev, false);
> +	mlx5_pci_disable_device(dev);
> +}
> +
> +static void mlx5_pci_reset_done(struct pci_dev *pdev)
> +{
> +	struct mlx5_core_dev *dev = pci_get_drvdata(pdev);
> +	struct devlink *devlink;
> +	int err;
> +
> +	if (!dev)
> +		return;
> +
> +	devlink = priv_to_devlink(dev);
> +
> +	if (!dev->pci_reset_in_progress) {
> +		mlx5_core_info(dev,
> +			       "%s: reset owned by fw_reset flow, leaving reload to it\n",
> +			       __func__);
> +		return;
> +	}

[Severity: High]
Will this early return prevent the driver from reloading after the external
sysfs reset mentioned above?

It seems this could leave the software state permanently out-of-sync with the
reset hardware.

> +
> +	err = mlx5_pci_enable_device(dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011094809.790798-1-tariqt@nvidia.com?part=1