Message ID | 20230911080828.635184-1-oushixiong@kylinos.cn (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | vfio/pds: Using pci_physfn() to fix a compilation issue | expand |
> From: oushixiong <oushixiong@kylinos.cn> > Sent: Monday, September 11, 2023 4:08 PM > > From: Shixiong Ou <oushixiong@kylinos.cn> > > If PCI_ATS isn't set, then pdev->physfn is not defined. > it causes a compilation issue: > > ../drivers/vfio/pci/pds/vfio_dev.c:165:30: error: ‘struct pci_dev’ has no > member named ‘physfn’; did you mean ‘is_physfn’? > 165 | __func__, pci_dev_id(pdev->physfn), pci_id, vf_id, > | ^~~~~~ > > So using pci_physfn() rather than using pdev->physfn directly. > > Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On Mon, Sep 11, 2023 at 08:25:18AM +0000, Tian, Kevin wrote: > > From: oushixiong <oushixiong@kylinos.cn> > > Sent: Monday, September 11, 2023 4:08 PM > > > > From: Shixiong Ou <oushixiong@kylinos.cn> > > > > If PCI_ATS isn't set, then pdev->physfn is not defined. > > it causes a compilation issue: > > > > ../drivers/vfio/pci/pds/vfio_dev.c:165:30: error: ‘struct pci_dev’ has no > > member named ‘physfn’; did you mean ‘is_physfn’? > > 165 | __func__, pci_dev_id(pdev->physfn), pci_id, vf_id, > > | ^~~~~~ > > > > So using pci_physfn() rather than using pdev->physfn directly. > > > > Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com> Yes, we should do both patches Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
On Mon, 11 Sep 2023 09:28:02 -0300 Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Mon, Sep 11, 2023 at 08:25:18AM +0000, Tian, Kevin wrote: > > > From: oushixiong <oushixiong@kylinos.cn> > > > Sent: Monday, September 11, 2023 4:08 PM > > > > > > From: Shixiong Ou <oushixiong@kylinos.cn> > > > > > > If PCI_ATS isn't set, then pdev->physfn is not defined. > > > it causes a compilation issue: > > > > > > ../drivers/vfio/pci/pds/vfio_dev.c:165:30: error: ‘struct pci_dev’ has no > > > member named ‘physfn’; did you mean ‘is_physfn’? > > > 165 | __func__, pci_dev_id(pdev->physfn), pci_id, vf_id, > > > | ^~~~~~ > > > > > > So using pci_physfn() rather than using pdev->physfn directly. > > > > > > Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn> > > > > Reviewed-by: Kevin Tian <kevin.tian@intel.com> > > Yes, we should do both patches Sure, but it's sloppy to put both in with the same commit log. Should we change this one to something like: vfio/pds: Use proper PF device access helper The pci_physfn() helper exists to support cases where the physfn field may not be compiled into the pci_dev structure. We've declared this driver dependent on PCI_IOV to avoid this problem, but regardless we should follow the precedent not to access this field directly. Thanks, Alex
diff --git a/drivers/vfio/pci/pds/vfio_dev.c b/drivers/vfio/pci/pds/vfio_dev.c index b46174f5eb09..649b18ee394b 100644 --- a/drivers/vfio/pci/pds/vfio_dev.c +++ b/drivers/vfio/pci/pds/vfio_dev.c @@ -162,7 +162,7 @@ static int pds_vfio_init_device(struct vfio_device *vdev) pci_id = PCI_DEVID(pdev->bus->number, pdev->devfn); dev_dbg(&pdev->dev, "%s: PF %#04x VF %#04x vf_id %d domain %d pds_vfio %p\n", - __func__, pci_dev_id(pdev->physfn), pci_id, vf_id, + __func__, pci_dev_id(pci_physfn(pdev)), pci_id, vf_id, pci_domain_nr(pdev->bus), pds_vfio); return 0;