pci_user: Report NUMA domain A PCI device's NUMA domain is now accessible via the pd_numa_domain member of struct pci_conf when using the PCIOCGETCONF ioctl. A new ioctl number has been assigned to PCIOCGETCONF to preserve compatibility with binaries compiled on FreeBSD versions 7 through 14. Such binaries can continue to use the PCIOCGETCONF ioctl number that they were compiled with and experience no ABI repercussions. Sponsored by: NIKSUN, Inc.
Details
Compiled the following program on FreeBSD 13 and FreeBSD 15.
https://reviews.freebsd.org/P644
The FreeBSD 13 version had the pd_numa_domain bit commented out, since it is unavailable.
Ran them both on FreeBSD 15 and diff'd the outputs. There was no difference in output besides numa domain.
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 56659 Build 53547: arc lint + arc unit
Event Timeline
These changes aren't backward compatible. You likely need to keep the old struct to reply to the old ioctl...
And you can't change the current old structure. Old binaries will have buffer overflows
Do not update pci_conf_old and pci_conf_old32 structures. These
should stay static for compatibility with binaries that were
compiled to use the PCIOCGETCONF_OLD and PCIOCGETCONF_OLD32 ioctls.
Thanks for removing this from all the old versions...
However, we still have some compatibility issue. I think we need to rename all the _old names to _freebsd7 to make it comply with more-modern conventions. This can likely be a separate commit.
Then, we need to take the current structures and make _freebsd14 compat versions for them. Since the layout is the same, we'd likely be able to do this by just not setting pd_numa_domain if the IOCTL (which encodes the lenght) is the _freebsd14 version I'd like you to create. It sounds like a lot, but I think most of it is cut and paste and isn't that bad since it's an extra case statement and an extra if for the normal and compat32 branches.
So something like https://reviews.freebsd.org/D45867 for the rename (I can commit this if you'd like)
And something like https://reviews.freebsd.org/D45868 for the compat stuff... It's a little different than I originally thought, so there will need to be a new ioctl to cover this case since the sizes stay the same, we have to change the number so we don't overflow the receiver's conf buffer if it's run against a new kernel. I suspect the new ioctl# and the freebsd14 ioctl parts from that differential belong here, and then you can take mine for compat, make sure it works and then commit it maybe (or if you want me to, let me know)
sys/sys/pciio.h | ||
---|---|---|
80 | Does it make sense to add some spare fields here, so that you can add additional fields later without needing compat shims? |
sys/sys/pciio.h | ||
---|---|---|
80 |
I was thinking the same thing - this is a lot of compat cruft for a single field. It would be unfortunate if we needed to do it all again for another field in the next major version. I do not know how this should be handled. Maybe some simple char padding would do? |
sys/sys/pciio.h | ||
---|---|---|
80 | For most 'compat' items, the length is just appended and it's just an extra line or two. |