On Tue, 16 Mar 2021 10:20:58 -0300 Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > On Tue, Mar 16, 2021 at 08:04:55AM +0000, Tian, Kevin wrote: > > > @@ -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
