Re: [PATCH] PCI: Add D3cold reset quirk for devices with broken/missing FLR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unconditional success return masks D3cold transition failures and bypasses device reset.
- [High] Failure to disable broken FLR exposes native drivers to device hangs.
- [Medium] Hardcoding the 'vfio-pci' driver name violates architectural boundaries and breaks with modern VFIO variants.
- [Medium] Inconsistent probe logic causes `pci_reset_supported()` to falsely advertise reset capability.
--

commit 2f9a723d1660924d07061cb5b2d479d4c40e48b5
Author: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>

PCI: Add D3cold reset quirk for devices with broken/missing FLR

This commit adds a D3cold->D0 power cycle reset quirk for several PCIe
devices lacking functional FLR when used in VFIO passthrough scenarios. It
targets specific Qualcomm and MediaTek devices to ensure they are properly
reset between VM assignments.

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c

[ ... ]

> +static int reset_device_d3cold(struct pci_dev *dev, bool probe)
> +{
> +	int ret;
> +
> +	if (probe)
> +		return 0;
> +
> +	if (!dev->driver || strcmp(dev->driver->name, "vfio-pci") != 0)
> +		return -ENOTTY;

Since the function unconditionally returns 0 when probe is true, but might
return -ENOTTY later if the device is not bound to vfio-pci, won't this cause
pci_reset_supported() to falsely advertise reset capability to upper layers
like sysfs?

Is it safe to rely on a strict string comparison with vfio-pci? Modern VFIO
setups often utilize variant drivers such as vfio-pci-core, mlx5_vfio_pci,
or nvgrace-gpu-vfio-pci, which would fail this check.

> +	/*
> +	 * D3cold->D0 power cycle for firmware reset.
> +	 * VFIO has already disabled interrupts and will handle state
> +	 * save/restore, so we just do the power transition.
> +	 */
> +	ret = pci_set_power_state(dev, PCI_D3cold);
> +	if (ret && ret != -EIO)
> +		pci_warn(dev, "D3cold transition failed: %d\n", ret);
> +
> +	ret = pci_set_power_state(dev, PCI_D0);
> +	if (ret && ret != -EIO)
> +		pci_warn(dev, "D0 transition failed: %d\n", ret);
> +
> +	pci_info(dev, "D3cold reset completed\n");
> +	return 0;
> +}

Does unconditionally returning 0 here mask transition failures? If
pci_set_power_state() fails and the device doesn't actually reach D3cold,
VFIO might assign an un-reset device to the next VM.

Should the function verify that the target state was reached and propagate
the error instead of proceeding?

>  static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
[ ... ]
>  	{ PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HINIC_VF,
>  		reset_hinic_vf_dev },
> +	{ PCI_VENDOR_ID_QCOM, 0x1103, reset_device_d3cold },     /* Qualcomm ath11k WiFi */
> +	{ PCI_VENDOR_ID_QCOM, 0x1107, reset_device_d3cold },     /* Qualcomm ath12k WiFi */
> +	{ PCI_VENDOR_ID_QCOM, 0x0308, reset_device_d3cold },     /* Qualcomm SDX62/SDX65 5G modem */
> +	{ PCI_VENDOR_ID_MEDIATEK, 0x7925, reset_device_d3cold }, /* MediaTek mt7925e WiFi */
>  	{ 0 }
>  };

The commit message notes that devices like the MediaTek mt7925e have an
incomplete or broken FLR. Since this patch doesn't set
PCI_DEV_FLAGS_NO_FLR_RESET, and reset_device_d3cold() returns -ENOTTY for
native drivers, won't the PCI core fall back to using the explicitly
broken FLR for native resets?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507142916.392983-1-jtornosm@redhat.com?part=1