search

nutanix / libvfio-user

framework for emulating devices in userspace
BSD 3-Clause "New" or "Revised" License
164 stars 51 forks source link

Allow adding MSI capability via vfu_pci_add_capability #758

Closed FlorianFreudiger closed 1 year ago

FlorianFreudiger commented 1 year ago

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 https://github.com/nutanix/libvfio-user/blob/8872c36d5047e464952d8aa7f3f12633357d758d/include/pci_caps/msi.h#L68 compared to: https://github.com/nutanix/libvfio-user/blob/8872c36d5047e464952d8aa7f3f12633357d758d/include/pci_caps/msix.h#L72

jlevon commented 1 year ago

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

FlorianFreudiger commented 1 year ago

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

FlorianFreudiger commented 1 year ago

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

https://github.com/nutanix/libvfio-user/blob/8872c36d5047e464952d8aa7f3f12633357d758d/lib/pci_caps.c#L195

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.

jlevon commented 1 year ago

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

https://github.com/nutanix/libvfio-user/blob/8872c36d5047e464952d8aa7f3f12633357d758d/lib/pci_caps.c#L195

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!

jlevon commented 1 year ago

@FlorianFreudiger please rebase and I'll merge