> From: Alex Williamson <alex.williamson@xxxxxxxxxx> > Sent: Wednesday, March 17, 2021 6:27 AM > > 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, > OK. Then, Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
