Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

Correct isPCIeDevice logic #2889

Merged
amshinde merged 1 commit into from
Aug 24, 2020
Merged

Correct isPCIeDevice logic #2889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions virtcontainers/device/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
38 changes: 12 additions & 26 deletions virtcontainers/device/drivers/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package drivers
import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"strings"

Expand All @@ -22,6 +23,8 @@ const (

PCIDomain = "0000"
PCIeKeyword = "PCIe"

PCIConfigSpaceSize = 256
)

type PCISysFsType string
Expand Down Expand Up @@ -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")
Copy link
Copy Markdown
Member

@fidencio fidencio Aug 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use WithError(err), instead of WithField("error", err), but that's a nitpick.

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
Copy link
Copy Markdown

@jodh-intel jodh-intel Aug 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to say it might be safer to check if the size is exactly 4096. But presumably if another PCI standard is released that would be a superset of PCI-e (as PCI-e is to PCI) so I guess this is "safe" as-is 😄

Copy link
Copy Markdown
Contributor Author

@dgibson dgibson Aug 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, and my guess would be that that hypothetical extension will probably behave closer to PCI express than classic PCI.

}

// read from /sys/bus/pci/devices/xxx/property
Expand All @@ -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
Expand Down