Feature: add a method for binding shared virtual addressing #3014
Conversation
Add fpgaBindSVA() to allocate and bind PASIDs to FPGA ports. Shared virtual addressing requires hardware support, both in the CPU and on the FPGA. fpgaBindSVA() depends on /dev/dfl-pci-sva from the dfl-pci driver. IOVA-based pinned DMA remains available with older drivers and hardware that does not support PASID.
Pull Request Test Coverage Report for Build 6243520861
💛 - Coveralls |
|
@rweight and @matthew-gerlach - These are the user space changes that go along with the dfl-pci driver updates. Note that the changes here will work with old drivers. They won't find /dev/dfl-pci-sva and will be fine with that. |
libraries/plugins/vfio/opae_vfio.c
Outdated
| if (h->sva_fd) { | ||
| // Release PASID and shared virtual addressing | ||
| opae_close(h->sva_fd); | ||
| h->sva_fd = 0; |
There was a problem hiding this comment.
0 is actually a valid file descriptor. Could we use -1 as the closed sentinel for this?
libraries/plugins/vfio/opae_vfio.c
Outdated
|
|
||
| h = handle_check_and_lock(handle); | ||
| ASSERT_NOT_NULL(h); | ||
| ASSERT_NOT_NULL(pasid); |
There was a problem hiding this comment.
If we want to allow pasid to be NULL, this will prevent that. Better to add a NULL check at 1621 and 1642.
libraries/plugins/vfio/opae_vfio.c
Outdated
| // then assume PASID is not supported, either by the host or the FPGA. | ||
| snprintf(path, sizeof(path), "/dev/dfl-pci-sva/%s", v->cont_pciaddr); | ||
| fd = opae_open(path, O_RDONLY); | ||
| if (fd > 0) { |
There was a problem hiding this comment.
| if (fd > 0) { | |
| if (fd >= 0) { |
0 is a valid file descriptor.
libraries/plugins/vfio/opae_vfio.c
Outdated
| if (fd > 0) { | ||
| // Request a shared virtual addressing. On success, the PASID is | ||
| // returned. | ||
| int res = opae_ioctl(fd, DFL_PCI_SVA_BIND_DEV); |
There was a problem hiding this comment.
Could we use a different name than res? res is also defined on 1607.
…ly if the module isn't available. - Improvements to file descriptor and PASID range checks.
|
@tswhison - I'm confused by the failed test at opae-sdk/tests/opae-v/test_opae_vfio_c.cpp:3246. I think I didn't change anything relevant. I think the test causes vfio_fpgaDestroyEventHandle() to close fd 0. Seems like a bad idea. |
of 0 don't look like valid handles.
|
@tswhison - I still think that opae-sdk/tests/opae-v/test_opae_vfio_c.cpp:3246 is a broken test since it closes fd 0. I exposed that bug by causing another test to do the same thing. My case is fixed in the latest commit by adding flags for pasid so that when the handle is initialized to 0 it doesn't look like there's a file open. |
|
Thanks for pointing this out. I will submit a fix. |
Description
Add fpgaBindSVA() to allocate and bind PASIDs to FPGA ports. Shared virtual addressing requires hardware support, both in the CPU and on the FPGA.
fpgaBindSVA() depends on /dev/dfl-pci-sva from the dfl-pci driver. IOVA-based pinned DMA remains available with older drivers and hardware that does not support PASID.
OPAE behavior is unchanged when fpgaBindSVA() is not called.
Collateral (docs, reports, design examples, case IDs):
Tests added:
Tests run: