Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for FPGAs connected via the vfio-platform driver #783

Draft
wants to merge 99 commits into
base: master
Choose a base branch
from
Draft

Add support for FPGAs connected via the vfio-platform driver #783

wants to merge 99 commits into from

Conversation

IgnoreWarnings
Copy link
Collaborator

@IgnoreWarnings IgnoreWarnings commented May 22, 2024 •
edited
Loading

DO NOT MERGE!
This is a draft only to discuss and track progress. Changes will come in smaller PRs.
Integration of platform based fpgas.

@IgnoreWarnings IgnoreWarnings marked this pull request as draft May 22, 2024 10:26
stv0g
stv0g requested changes May 22, 2024
View reviewed changes
Copy link
Contributor

@stv0g stv0g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a first round of review. I will need to go a bit deeper into the actual logic.

Some general things:

  • Please reformat all your changes with clang-format
  • Make sure your comments have a DCO sign-off (the CI check is failing)
  • Please try to consolitate/stash your commits in a way it makes the code easier to review. E.g. not everything into one commit, but smaller groups of changes which are logically belong together.

.gitignore Outdated Show resolved Hide resolved
etc/examples/nodes/miob.conf Outdated Show resolved Hide resolved
etc/examples/nodes/miob.conf Outdated Show resolved Hide resolved
etc/examples/nodes/miob.conf Outdated Show resolved Hide resolved
fpga/src/platform.cpp Outdated Show resolved Hide resolved
fpga/scripts/bind_platform_vfio.sh Outdated Show resolved Hide resolved
fpga/src/platform.cpp Outdated Show resolved Hide resolved
fpga/src/platform.cpp Outdated Show resolved Hide resolved
fpga/lib/ips/zynq.cpp Outdated Show resolved Hide resolved
fpga/lib/ips/platform_intc.cpp Outdated Show resolved Hide resolved
@stv0g stv0g changed the title Support Platform Bus Fpgas Add support FPGAs connected via VFIO platform drivers May 22, 2024
@n-eiling
Copy link
Member

n-eiling commented May 22, 2024 •
edited
Loading

This is just a draft to track the progress. It's clear that it needs squashing, signing-off and formatting. It may also make sense to split it into multiple PRs.

@n-eiling n-eiling changed the title Add support FPGAs connected via VFIO platform drivers Add support for FPGAs connected via the vfio-platform driver May 22, 2024
@stv0g
Copy link
Contributor

stv0g commented May 23, 2024

Oh okay, sorry I did not see that this PR was a draft.

@IgnoreWarnings
Copy link
Collaborator Author

Oh okay, sorry I did not see that this PR was a draft.

I accidentally opened a PR at first and then changed to draft.
@n-eiling suggested i open a draft to track the progress and announce the feature.

Since this is work in progress, many of the changes requested are already outdated/fixed.
Please wait for the PR to comment on (smaller) issues, because many of them will be resolved till then.

@IgnoreWarnings IgnoreWarnings force-pushed the platform branch 2 times, most recently from 5b5091e to e2cd4d8 Compare June 5, 2024 09:40
@IgnoreWarnings IgnoreWarnings force-pushed the platform branch 6 times, most recently from ef5fd45 to 45f732b Compare June 11, 2024 11:36
n-eiling
n-eiling requested changes Jun 11, 2024
View reviewed changes
common/lib/kernel/vfio_device.cpp Show resolved Hide resolved
etc/examples/nodes/miob.conf Outdated Show resolved Hide resolved
etc/examples/nodes/miob.conf Outdated Show resolved Hide resolved
etc/fpga/miob/fpgas.json Outdated Show resolved Hide resolved
etc/fpga/miob/ips.json Outdated Show resolved Hide resolved
fpga/lib/ips/zynq.cpp Outdated
// Save ID in card so we can create mappings later when needed (e.g. when
// allocating DMA memory in host RAM)
card->addrSpaceIdDeviceToHost =
mm.getOrCreateAddressSpace("zynq_ultra_ps_e_0/HPC0_DDR_LOW");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this need to be hardcoded?

fpga/scripts/bind_platform_vfio.sh Outdated Show resolved Hide resolved
fpga/src/CMakeLists.txt Outdated Show resolved Hide resolved
fpga/src/platform.cpp Outdated Show resolved Hide resolved
lib/nodes/fpga.cpp Outdated Show resolved Hide resolved
@IgnoreWarnings IgnoreWarnings mentioned this pull request Jun 17, 2024
Merged
@IgnoreWarnings IgnoreWarnings force-pushed the platform branch 2 times, most recently from 6a5ec9d to 5f3568c Compare July 1, 2024 13:48
@IgnoreWarnings IgnoreWarnings mentioned this pull request Jul 18, 2024
Closed
@IgnoreWarnings IgnoreWarnings force-pushed the platform branch 3 times, most recently from d8ddbb4 to 1ebb82c Compare August 2, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n-eiling n-eiling n-eiling requested changes

@stv0g stv0g Awaiting requested review from stv0g stv0g is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@IgnoreWarnings @n-eiling @stv0g