Details

Reviewers

jhb
bryanv
number201724_me.com
philip

Commits

rG6fe25201ea62: virtio_pci_legacy: Use the table BAR and PBA BAR from MSI-X cap
rG98740ead97c6: virtio_pci_legacy: Use the table BAR and PBA BAR from MSI-X cap
rGfaf9a4e9142a: virtio_pci_legacy: Use the table BAR and PBA BAR from MSI-X cap

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

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 37293
Build 34182: arc lint + arc unit

Event Timeline

khng created this revision.Feb 20 2021, 5:39 PM

Herald added a subscriber: imp. · View Herald TranscriptFeb 20 2021, 5:39 PM

khng requested review of this revision.Feb 20 2021, 5:39 PM

Harbormaster completed remote builds in B37219: Diff 84334.Feb 20 2021, 5:39 PM

@number201724_me.com Could you test this patch on FreeBSD 13 BETA2? Thanks.

khng added a child revision: D28818: virtio_pci_legacy: Allow memory space for configuration.Feb 20 2021, 6:11 PM

khng mentioned this in D28818: virtio_pci_legacy: Allow memory space for configuration.Feb 20 2021, 6:17 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

bryanv added inline comments.Feb 23 2021, 5:04 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

Harbormaster completed remote builds in B37293: Diff 84516.Feb 23 2021, 7:39 AM

khng marked an inline comment as done.Feb 23 2021, 7:41 AM

bryanv accepted this revision.Feb 23 2021, 1:55 PM

This revision is now accepted and ready to land.Feb 23 2021, 1:55 PM

khng planned changes to this revision.Feb 25 2021, 10:56 AM

Fix compilation.

Harbormaster completed remote builds in B37364: Diff 84665.Feb 25 2021, 11:00 AM

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

jhb accepted this revision.Feb 25 2021, 7:59 PM

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

After testing, this patch runs normally on alibabacloud

Nit: naming

This revision now requires review to proceed.Mar 2 2021, 4:25 PM

Harbormaster completed remote builds in B37503: Diff 84971.Mar 2 2021, 4:26 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.

khng added a reviewer: philip.Mar 19 2021, 6:02 AM

Approved by: philip (mentor)

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

Closed by commit rGfaf9a4e9142a: virtio_pci_legacy: Use the table BAR and PBA BAR from MSI-X cap (authored by khng). · Explain WhyMar 20 2021, 7:46 AM

This revision was automatically updated to reflect the committed changes.

khng added a commit: rGfaf9a4e9142a: virtio_pci_legacy: Use the table BAR and PBA BAR from MSI-X cap.

khng added a commit: rG98740ead97c6: virtio_pci_legacy: Use the table BAR and PBA BAR from MSI-X cap.Apr 3 2021, 6:21 AM

markj added a commit: rG6fe25201ea62: virtio_pci_legacy: Use the table BAR and PBA BAR from MSI-X cap.May 26 2021, 8:38 PM