Page MenuHomeFreeBSD

Create a separate interface for leaf-driver PCI IOV methods.
ClosedPublic

Authored by jhb on May 19 2015, 3:05 PM.

Details

Summary

Create a separate interface for leaf-driver PCI IOV methods.

Leaf drivers should not import the PCI bus interface to add IOV handling.
Instead, move the IOV client methods to a separate kobj interface.

Test Plan
  • Not yet tested. I will do a universe build, but I don't have an easy way to runtime test this. I ran this by Ryan a while ago, but it would be nice to fix this sooner rather than later.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb retitled this revision from to Create a separate interface for leaf-driver PCI IOV methods..
jhb updated this object.
jhb edited the test plan for this revision. (Show Details)
jhb added a reviewer: rstone.
jhb added subscribers: erj, jfv.

Note that I have a separate review to remove the executable property from the ixl source, but arcanist wouldn't let me upload the change to if_ixl.c without that change.

Please update the PCI_INIT_IOV(9), PCI_ADD_VF(9) and PCI_UNINIT_IOV(9) manpages to document the new header file and reflect the new method names.

sys/dev/pci/pci_iov.c
704 ↗(On Diff #5479)

I think that you meant PCI_IOV_UNINIT() here

sys/dev/pci/pci_iov_if.m
2 ↗(On Diff #5479)

Should this file still remain "Copyright (c) 1998 Doug Rabson"?

I believe that you need to update sys/modules/ixl/Makefile to document that the driver now depends on pci_iov_if.h as well

jhb marked an inline comment as done.
jhb edited edge metadata.

Rebase.

  • Rename manual pages.
  • Catch up to manpage renames.
  • Fix a IOV_UINIT -> PCI_IOV_UINIT I missed.

Still need to do a universe build which should flesh out the ixl Makefile as well.

sys/dev/pci/pci_iov_if.m
2 ↗(On Diff #5479)

It should probably get a license from Ryan (or Ryan's employer)

share/man/man9/PCI_IOV_ADD_VF.9
35 ↗(On Diff #5748)

We probably should document that "pci_iov_if.h" needs to be included to implement the method?

sys/dev/pci/pci_iov_if.m
2 ↗(On Diff #5748)

Making it "Copyright Sandvine Inc." to match the other SR-IOV files would make the most sense

jhb marked an inline comment as done.May 28 2015, 6:53 PM
jhb added inline comments.
share/man/man9/PCI_IOV_ADD_VF.9
35 ↗(On Diff #5748)

Yes, though we generally don't require drivers to explicitly include foo_if.h headers. We normally have a wrapper header that includes them (e.g. sys/bus.h pulls in device_if.h and bus_if.h, and pcivar.h pulls in pci_if.h). I think it would be too disruptive to make pcivar.h pull his header in since we'd have to change umpteen different module Makefiles. One option I had thought about was creating a sys/dev/pci/pci_iov.h and moving the pci_iov_attach/detach wrapper functions into that. It would also have the nested include of pci_iov_if.h.

sys/dev/pci/pci_iov_if.m
2 ↗(On Diff #5748)

One oddity is that the pci_iov.c file (at least, haven't checked the others) has the phrase "All rights reserved" duplicated.

share/man/man9/PCI_IOV_ADD_VF.9
35 ↗(On Diff #5748)

that sounds fine

sys/dev/pci/pci_iov_if.m
2 ↗(On Diff #5748)

That's funny. I'll fix it in -head tonight

jhb marked an inline comment as done.
jhb edited edge metadata.
  • Move pci_iov_* prototypes into pci_iov.h and include pci_iov_if.h there.
  • Fix ixl build.
share/man/man9/PCI_IOV_ADD_VF.9
35 ↗(On Diff #5757)

Ok, I've done that. I think that in theory a driver would only need to include pci_iov.h and not
the var and reg.h headers if all it did was IOV so I've only listed those. Of course, any real driver needs a probe routine, etc. and those will need the other headers, but not the IOV-specific bits I think.

sys/dev/pci/pci_iov.h
2 ↗(On Diff #5757)

Er, you can omit the redundant "All rights reserved". :)

rstone edited edge metadata.
This revision is now accepted and ready to land.May 28 2015, 9:30 PM
jhb edited edge metadata.
  • License bogon.
This revision now requires review to proceed.May 28 2015, 9:51 PM
jhb added a reviewer: jhb.
This revision is now accepted and ready to land.May 28 2015, 9:52 PM
This revision was automatically updated to reflect the committed changes.