On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst at redhat.com> wrote:>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int >> num_vfs) >> +{ >> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); >> + struct virtio_device *vdev = &vp_dev->vdev; >> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs); >> + >> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK)) >> + return -EBUSY; >> + >> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV)) >> + return -EINVAL; >> + >> + sriov_configure = pci_sriov_configure_simple; >> + if (sriov_configure == NULL) >> + return -ENOENT; > > BTW what is all this trickery in aid of?When SR-IOV support is not compiled into the kernel, pci_sriov_configure_simple is #defined as NULL. This allows it to compile in that case, even though there is utterly no way for it to be called in that case. It is an alternative to #ifs in the code. -- Mark Rustad, Networking Division, Intel Corporation -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 873 bytes Desc: Message signed with OpenPGP URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20180530/1cbeee6e/attachment.sig>
On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote:> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst at redhat.com> wrote: > > > > +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int > > > num_vfs) > > > +{ > > > + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); > > > + struct virtio_device *vdev = &vp_dev->vdev; > > > + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs); > > > + > > > + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK)) > > > + return -EBUSY; > > > + > > > + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV)) > > > + return -EINVAL; > > > + > > > + sriov_configure = pci_sriov_configure_simple; > > > + if (sriov_configure == NULL) > > > + return -ENOENT; > > > > BTW what is all this trickery in aid of? > > When SR-IOV support is not compiled into the kernel, > pci_sriov_configure_simple is #defined as NULL. This allows it to compile > in that case, even though there is utterly no way for it to be called in > that case. It is an alternative to #ifs in the code.Why even have the call though? I would wrap all of this in an #ifdef and strip it out since you couldn't support SR-IOV if it isn't present in the kernel anyway.
On May 30, 2018, at 9:54 AM, Duyck, Alexander H <alexander.h.duyck at intel.com> wrote:> On Wed, 2018-05-30 at 09:44 -0700, Rustad, Mark D wrote: >> On May 30, 2018, at 9:22 AM, Michael S. Tsirkin <mst at redhat.com> wrote: >> >>>> +static int virtio_pci_sriov_configure(struct pci_dev *pci_dev, int >>>> num_vfs) >>>> +{ >>>> + struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev); >>>> + struct virtio_device *vdev = &vp_dev->vdev; >>>> + int (*sriov_configure)(struct pci_dev *pci_dev, int num_vfs); >>>> + >>>> + if (!(vdev->config->get_status(vdev) & VIRTIO_CONFIG_S_DRIVER_OK)) >>>> + return -EBUSY; >>>> + >>>> + if (!__virtio_test_bit(vdev, VIRTIO_F_SR_IOV)) >>>> + return -EINVAL; >>>> + >>>> + sriov_configure = pci_sriov_configure_simple; >>>> + if (sriov_configure == NULL) >>>> + return -ENOENT; >>> >>> BTW what is all this trickery in aid of? >> >> When SR-IOV support is not compiled into the kernel, >> pci_sriov_configure_simple is #defined as NULL. This allows it to compile >> in that case, even though there is utterly no way for it to be called in >> that case. It is an alternative to #ifs in the code. > > Why even have the call though? I would wrap all of this in an #ifdef > and strip it out since you couldn't support SR-IOV if it isn't present > in the kernel anyway.I am inclined to agree. In this case, the presence of #ifdefs I think would be clearer. As written, someone will want to get rid of the pointer only to create a build problem when SR-IOV is not configured. -- Mark Rustad, Networking Division, Intel Corporation -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 873 bytes Desc: Message signed with OpenPGP URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20180530/e3f40867/attachment.sig>