Page MenuHomeFreeBSD

virtio_pci_legacy: Use the table BAR and PBA BAR from MSI-X cap
ClosedPublic

Authored by khng on Feb 20 2021, 5:39 PM.

Details

Summary

The MSI-X resource shouldn't be assumed to be always on BAR1.
The Virtio v1.1 Spec did not specify that MSI-X table and PBA BAR has to
be BAR1 either.

Reported by: Yuan Rui <number201724@me.com>
MFC after: 2 weeks

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

khng requested review of this revision.Feb 20 2021, 5:39 PM
bryanv requested changes to this revision.Feb 23 2021, 5:03 AM
This revision now requires changes to proceed.Feb 23 2021, 5:03 AM
sys/dev/virtio/pci/virtio_pci_legacy.c
234

This error message isn't necessarily correct, and if the MSIX allocs fail, we don't release BAR 0 (both are existing issues before this change).

Moving the MSIX stuff out of vtpci_legacy_alloc_resources() and calling vtpci_legacy_setup_msix() from this function probably fits the existing flow the best, so we can jump to fail to do the cleanup.

  • Moved virtio_legacy_setup_msix to virtio_legacy_attach
  • Moved virtio_legacy_cleanup_msix to virtio_legacy_detach
khng marked an inline comment as done.Feb 23 2021, 7:41 AM
This revision is now accepted and ready to land.Feb 23 2021, 1:55 PM

I think this looks fine. Minor nit: I think we tend to use 'teardown' or 'free' more commonly in device drivers rather than 'cleanup'.

This revision is now accepted and ready to land.Feb 25 2021, 7:59 PM

After testing, this patch runs normally on alibabacloud

This revision now requires review to proceed.Mar 2 2021, 4:25 PM
In D28817#647763, @jhb wrote:

I think this looks fine. Minor nit: I think we tend to use 'teardown' or 'free' more commonly in device drivers rather than 'cleanup'.

Changed to match our naming convention.

Approved by: philip (mentor)

This revision is now accepted and ready to land.Mar 20 2021, 6:47 AM