On Tue, 16 Mar 2021 10:20:58 -0300
Jason Gunthorpe [off-list ref] wrote:

On Tue, Mar 16, 2021 at 08:04:55AM +0000, Tian, Kevin wrote:

quoted

quoted

@@ -2060,15 +2056,20 @@ static int vfio_pci_probe(struct pci_dev *pdev,

const struct pci_device_id *id)
 		vfio_pci_set_power_state(vdev, PCI_D3hot);
 	}

-	return ret;
+	ret = vfio_add_group_dev(&pdev->dev, &vfio_pci_ops, vdev);
+	if (ret)
+		goto out_power;
+	return 0;

+out_power:
+	if (!disable_idle_d3)
+		vfio_pci_set_power_state(vdev, PCI_D0);  

Just curious whether the power state must be recovered upon failure here.
From the comment several lines above, the power state is set to an unknown
state before doing D3 transaction. From this point it looks fine if leaving the
device in D3 since there is no expected state to be recovered?  

I don't know, this is what the remove function does, so I can't see a
reason why remove should do it but not here.

I'm not sure it matters in either case, we're just trying to be most
similar to expected driver behavior.  pci_enable_device() puts the
device in D0 but pci_disable_device() doesn't touch the power state, so
the device would typically be released from a PCI driver in D0 afaict.
Thanks,

Alex