From 6bf93b23159e3f8eab62e9c2fc26ed9946a818e9 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 19 Aug 2020 16:06:22 +1000 Subject: [PATCH] drivers: Correct isPCIeDevice logic Currently, isPCIeDevice() attempts to determine if a (host) device is PCI-Express capable by looking up its link speed via the PCI slots information in sysfs. This is a) complicated and b) wrong. PCI-e devices don't have to have slots information, so this frequently fails. Instead determine if devices are PCI-e by checking for the presence of PCIe extended configuration space by looking at the size of the "config" file in sysfs. Fixes: #2678 Signed-off-by: David Gibson --- virtcontainers/device/config/config.go | 3 -- virtcontainers/device/drivers/utils.go | 38 ++++++++------------------ 2 files changed, 12 insertions(+), 29 deletions(-) diff --git a/virtcontainers/device/config/config.go b/virtcontainers/device/config/config.go index 5f73cf7d8c..6146e8885d 100644 --- a/virtcontainers/device/config/config.go +++ b/virtcontainers/device/config/config.go @@ -89,9 +89,6 @@ var SysIOMMUPath = "/sys/kernel/iommu_groups" // SysBusPciDevicesPath is static string of /sys/bus/pci/devices var SysBusPciDevicesPath = "/sys/bus/pci/devices" -// SysBusPciSlotsPath is static string of /sys/bus/pci/slots -var SysBusPciSlotsPath = "/sys/bus/pci/slots" - var getSysDevPath = getSysDevPathImpl // DeviceInfo is an embedded type that contains device data common to all types of devices. diff --git a/virtcontainers/device/drivers/utils.go b/virtcontainers/device/drivers/utils.go index ad046ca10b..72e27a5e21 100644 --- a/virtcontainers/device/drivers/utils.go +++ b/virtcontainers/device/drivers/utils.go @@ -9,6 +9,7 @@ package drivers import ( "fmt" "io/ioutil" + "os" "path/filepath" "strings" @@ -22,6 +23,8 @@ const ( PCIDomain = "0000" PCIeKeyword = "PCIe" + + PCIConfigSpaceSize = 256 ) type PCISysFsType string @@ -52,23 +55,17 @@ func isPCIeDevice(bdf string) bool { if len(strings.Split(bdf, ":")) == 2 { bdf = PCIDomain + ":" + bdf } - slots, err := ioutil.ReadDir(config.SysBusPciSlotsPath) + + configPath := filepath.Join(config.SysBusPciDevicesPath, bdf, "config") + fi, err := os.Stat(configPath) if err != nil { - deviceLogger().WithError(err).WithField("path", config.SysBusPciSlotsPath).Warn("failed to list pci slots") - return false - } - b := strings.Split(bdf, ".")[0] - for _, slot := range slots { - address := getPCISlotProperty(slot.Name(), PCISysFsSlotsAddress) - if b == address { - maxBusSpeed := getPCISlotProperty(slot.Name(), PCISysFsSlotsMaxBusSpeed) - if strings.Contains(maxBusSpeed, PCIeKeyword) { - return true - } - } + deviceLogger().WithField("dev-bdf", bdf).WithField("error", err).Warning("Couldn't stat() configuration space file") + return false //Who knows? } - deviceLogger().WithField("dev-bdf", bdf).Debug("can not find slot for bdf of pci device") - return false + + // Plain PCI devices hav 256 bytes of configuration space, + // PCI-Express devices have 4096 bytes + return fi.Size() > PCIConfigSpaceSize } // read from /sys/bus/pci/devices/xxx/property @@ -85,17 +82,6 @@ func getPCIDeviceProperty(bdf string, property PCISysFsProperty) string { return rlt } -// read from /sys/bus/pci/slots/xxx/property -func getPCISlotProperty(slot string, property PCISysFsProperty) string { - propertyPath := filepath.Join(config.SysBusPciSlotsPath, slot, string(property)) - rlt, err := readPCIProperty(propertyPath) - if err != nil { - deviceLogger().WithError(err).WithField("path", propertyPath).Warn("failed to read pci slot property") - return "" - } - return rlt -} - func readPCIProperty(propertyPath string) (string, error) { var ( buf []byte