| Message ID | 20221031123319.21532-1-akihiko.odaki@daynix.com |
|---|---|
| Headers | show |
| Series | pci: Abort if pci_add_capability fails | expand |
On Mon, Oct 31, 2022 at 09:33:02PM +0900, Akihiko Odaki wrote: > pci_add_capability appears most PCI devices. Its error handling required > lots of code, and led to inconsistent behaviors such as: > - passing error_abort > - passing error_fatal > - asserting the returned value > - propagating the error to the caller > - skipping the rest of the function > - just ignoring Thanks for the patchset! I don't think I'll be merging it for this merge window, the benefit seems small and chance of regressions high. I will tag this but pls remind me after the freeze to help make sure I do not lose it. > The code generating errors in pci_add_capability had a comment which > says: > > Verify that capabilities don't overlap. Note: device assignment > > depends on this check to verify that the device is not broken. > > Should never trigger for emulated devices, but it's helpful for > > debugging these. > > Indeed vfio has some code that passes capability offsets and sizes from > a physical device, but it explicitly pays attention so that the > capabilities never overlap and the only exception are MSI and MSI-X > capabilities. Therefore, we can add code specific to the case, and > always assert that capabilities never overlap in the other cases, > resolving these inconsistencies. > > v6: > - Error in case of MSI/MSI-X capability overlap (Alex Williamson) > > v5: > - Fix capability ID specification in vfio_msi_early_setup (Alex Williamson) > - Use range_covers_byte() (Alex Williamson) > - warn_report() in case of MSI/MSI-X capability overlap (Alex Williamson) > > v4: > - Fix typos in messages (Markus Armbruster) > - hw/vfio/pci: Ensure MSI and MSI-X do not overlap (Alex Williamson) > > v3: > - Correct patch split between virtio-pci and pci (Markus Armbruster) > - Add messages for individual patches (Markus Armbruster) > - Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > Akihiko Odaki (17): > hw/vfio/pci: Ensure MSI and MSI-X do not overlap > pci: Allow to omit errp for pci_add_capability > hw/i386/amd_iommu: Omit errp for pci_add_capability > ahci: Omit errp for pci_add_capability > e1000e: Omit errp for pci_add_capability > eepro100: Omit errp for pci_add_capability > hw/nvme: Omit errp for pci_add_capability > msi: Omit errp for pci_add_capability > hw/pci/pci_bridge: Omit errp for pci_add_capability > pcie: Omit errp for pci_add_capability > pci/shpc: Omit errp for pci_add_capability > msix: Omit errp for pci_add_capability > pci/slotid: Omit errp for pci_add_capability > hw/pci-bridge/pcie_pci_bridge: Omit errp for pci_add_capability > hw/vfio/pci: Omit errp for pci_add_capability > virtio-pci: Omit errp for pci_add_capability > pci: Remove legacy errp from pci_add_capability > > docs/pcie_sriov.txt | 4 +- > hw/display/bochs-display.c | 4 +- > hw/i386/amd_iommu.c | 21 ++------- > hw/ide/ich.c | 8 +--- > hw/net/e1000e.c | 22 ++------- > hw/net/eepro100.c | 7 +-- > hw/nvme/ctrl.c | 14 +----- > hw/pci-bridge/cxl_downstream.c | 9 +--- > hw/pci-bridge/cxl_upstream.c | 8 +--- > hw/pci-bridge/i82801b11.c | 14 +----- > hw/pci-bridge/pci_bridge_dev.c | 2 +- > hw/pci-bridge/pcie_pci_bridge.c | 19 ++------ > hw/pci-bridge/pcie_root_port.c | 16 +------ > hw/pci-bridge/xio3130_downstream.c | 15 ++---- > hw/pci-bridge/xio3130_upstream.c | 15 ++---- > hw/pci-host/designware.c | 3 +- > hw/pci-host/xilinx-pcie.c | 4 +- > hw/pci/msi.c | 9 +--- > hw/pci/msix.c | 8 +--- > hw/pci/pci.c | 29 +++--------- > hw/pci/pci_bridge.c | 21 +++------ > hw/pci/pcie.c | 52 ++++++--------------- > hw/pci/shpc.c | 23 +++------ > hw/pci/slotid_cap.c | 8 +--- > hw/usb/hcd-xhci-pci.c | 3 +- > hw/vfio/pci-quirks.c | 15 ++---- > hw/vfio/pci.c | 75 ++++++++++++++++++++++-------- > hw/vfio/pci.h | 3 ++ > hw/virtio/virtio-pci.c | 12 ++--- > include/hw/pci/pci.h | 5 +- > include/hw/pci/pci_bridge.h | 5 +- > include/hw/pci/pcie.h | 11 ++--- > include/hw/pci/shpc.h | 3 +- > include/hw/virtio/virtio-pci.h | 2 +- > 34 files changed, 153 insertions(+), 316 deletions(-) > > -- > 2.38.1