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.

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

Allow adding MSI capability via vfu_pci_add_capability #758

Merged
merged 6 commits into from
Aug 15, 2023

Conversation

FlorianFreudiger
Copy link
Contributor

Currently vfu_pci_add_capability(...) does not allow adding the MSI capability, only MSIX and a handful of other capabilities, even though the MSI capability definition is already present in include/pci_caps/msi.h.

The new cap_write_msi callback function is copied from cap_write_msix and adjusted to msicap, omitting the "function mask" bit which is not present in MSI's message control.

The size of the MSI capability does not seem to be defined in pci_regs, just like in

_Static_assert(sizeof(struct msicap) == 0x18, "bad MSICAP size");

compared to:
_Static_assert(sizeof(struct msixcap) == PCI_CAP_MSIX_SIZEOF, "bad MSI-X size");

@jlevon
Copy link
Collaborator

jlevon commented Aug 4, 2023

Hi Florian, thanks for your contribution! Would you mind looking at adding a pytest for this? Let me know if any questions.

@FlorianFreudiger FlorianFreudiger marked this pull request as draft August 5, 2023 14:04
@FlorianFreudiger
Copy link
Contributor Author

FlorianFreudiger commented Aug 5, 2023

I noticed there are some other fields that will need to be included in cap_write_msi

@FlorianFreudiger
Copy link
Contributor Author

I also found a small bug in the existing cap_write_msix I copied, it logs the opposite MSI-X enable bit.

msix->mxc.mxe ? "enable" : "disable");

it should either check new_msix.mxc.mxe instead of msix->mxc.mxe or the assignment should be performed before the log, similar to the other write functions.

@jlevon Let me know if I should include the small fix in this PR.

@FlorianFreudiger FlorianFreudiger marked this pull request as ready for review August 5, 2023 16:42
@jlevon
Copy link
Collaborator

jlevon commented Aug 5, 2023

I also found a small bug in the existing cap_write_msix I copied, it logs the opposite MSI-X enable bit.

msix->mxc.mxe ? "enable" : "disable");

it should either check new_msix.mxc.mxe instead of msix->mxc.mxe or the assignment should be performed before the log, similar to the other write functions.

@jlevon Let me know if I should include the small fix in this PR.

good spot! Please open a separate PR for that fix, thank you!

Copy link
Collaborator

@jlevon jlevon left a comment

Choose a reason for hiding this comment

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

thanks! Just one tiny nit

lib/pci_caps.c Outdated Show resolved Hide resolved
@jlevon
Copy link
Collaborator

jlevon commented Aug 15, 2023

@FlorianFreudiger please rebase and I'll merge

Signed-off-by: Florian Freudiger <25648113+FlorianFreudiger@users.noreply.github.com>
Signed-off-by: Florian Freudiger <25648113+FlorianFreudiger@users.noreply.github.com>
Signed-off-by: Florian Freudiger <25648113+FlorianFreudiger@users.noreply.github.com>
Signed-off-by: Florian Freudiger <25648113+FlorianFreudiger@users.noreply.github.com>
Signed-off-by: Florian Freudiger <25648113+FlorianFreudiger@users.noreply.github.com>
Signed-off-by: Florian Freudiger <25648113+FlorianFreudiger@users.noreply.github.com>
@jlevon jlevon merged commit 1cca91a into nutanix:master Aug 15, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants