diff --git a/Dockerfile b/Dockerfile index 0bf768a43..7e1e2f4c3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,12 +12,16 @@ ARG APT_MIRROR ARG APT_PORTS_MIRROR ENV DEBIAN_FRONTEND=noninteractive +# hwdata ships /usr/share/hwdata/pci.ids. Without it, the ghw library we use +# for hardware detection cannot resolve PCI vendor IDs and fails to enumerate +# GPUs at all, so the image reports "No GPU detected" (see issue #10941). RUN --mount=type=bind,source=.docker/apt-mirror.sh,target=/usr/local/sbin/apt-mirror \ APT_MIRROR="${APT_MIRROR}" APT_PORTS_MIRROR="${APT_PORTS_MIRROR}" sh /usr/local/sbin/apt-mirror && \ apt-get update && \ apt-get install -y --no-install-recommends \ ca-certificates curl wget espeak-ng libgomp1 \ - ffmpeg libopenblas0 libopenblas-dev libopus0 sox && \ + ffmpeg libopenblas0 libopenblas-dev libopus0 sox \ + hwdata && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* diff --git a/pkg/xsysinfo/gpu.go b/pkg/xsysinfo/gpu.go index 3166336d1..4583732b7 100644 --- a/pkg/xsysinfo/gpu.go +++ b/pkg/xsysinfo/gpu.go @@ -213,20 +213,21 @@ func minNonZeroVRAM(infos []GPUMemoryInfo) uint64 { return min } +// HasGPU reports whether a GPU of the given vendor is present. An empty +// vendor asks whether the host has any GPU at all. +// +// Both legs consult sysfs as well as ghw: ghw enumeration fails outright +// on images with no pci.ids database, and returning "no GPU" there is +// what made a passed-through Intel Arc invisible in #10941. func HasGPU(vendor string) bool { - gpus, err := GPUs() - if err != nil { - return false - } if vendor == "" { - return len(gpus) > 0 - } - for _, gpu := range gpus { - if strings.Contains(gpu.String(), vendor) { + gpus, err := GPUs() + if err == nil && len(gpus) > 0 { return true } + return len(scanSysfsGPUs(defaultSysfsDRMPath)) > 0 } - return false + return hasGPUVendor(vendor) } // DetectGPUVendor detects the GPU vendor using multiple methods with fallbacks. @@ -235,25 +236,18 @@ func HasGPU(vendor string) bool { // Priority order: NVIDIA > AMD > Intel > Vulkan func DetectGPUVendor() (string, error) { // First, try ghw library detection - gpus, err := GPUs() - if err == nil && len(gpus) > 0 { - for _, gpu := range gpus { - if gpu.DeviceInfo != nil && gpu.DeviceInfo.Vendor != nil { - vendorName := strings.ToUpper(gpu.DeviceInfo.Vendor.Name) - if strings.Contains(vendorName, strings.ToUpper(VendorNVIDIA)) { - xlog.Debug("GPU vendor detected via ghw", "vendor", VendorNVIDIA) - return VendorNVIDIA, nil - } - if strings.Contains(vendorName, strings.ToUpper(VendorAMD)) { - xlog.Debug("GPU vendor detected via ghw", "vendor", VendorAMD) - return VendorAMD, nil - } - if strings.Contains(vendorName, strings.ToUpper(VendorIntel)) { - xlog.Debug("GPU vendor detected via ghw", "vendor", VendorIntel) - return VendorIntel, nil - } - } - } + if vendor := vendorFromGHW(ghwCards()); vendor != "" { + xlog.Debug("GPU vendor detected via ghw", "vendor", vendor) + return vendor, nil + } + + // Then read PCI vendor IDs straight from sysfs. ghw needs a pci.ids + // database file to resolve vendor names and errors out entirely when + // the image doesn't ship one, which is how a passed-through Intel Arc + // ended up undetected in #10941. + if vendor := sysfsVendorPriority(defaultSysfsDRMPath); vendor != "" { + xlog.Debug("GPU vendor detected via sysfs", "vendor", vendor) + return vendor, nil } // Fallback to binary detection (priority: NVIDIA > AMD > Intel > Vulkan) @@ -707,9 +701,9 @@ func getIntelGPUMemory() []GPUMemoryInfo { return gpus } // clinfo enumerates every OpenCL platform, so guard the - // subprocess with the cached ghw GPU list: non-Intel hosts skip + // subprocess with the detected GPU list: non-Intel hosts skip // it entirely. - if !hasGHWVendor(VendorIntel) { + if !hasGPUVendor(VendorIntel) { return nil } var out []GPUMemoryInfo @@ -721,17 +715,28 @@ func getIntelGPUMemory() []GPUMemoryInfo { return out } -// hasGHWVendor reports whether ghw observed any GPU whose vendor name -// matches (case-insensitive substring). Uses the package-level cache -// in GPUs() so the call is free after the first invocation. -func hasGHWVendor(vendor string) bool { - gpus, _ := GPUs() - target := strings.ToUpper(vendor) - for _, g := range gpus { - if g.DeviceInfo == nil || g.DeviceInfo.Vendor == nil { - continue - } - if strings.Contains(strings.ToUpper(g.DeviceInfo.Vendor.Name), target) { +// ghwCards returns the GPUs ghw observed, or nil when it could not +// enumerate at all, which it does whenever no pci.ids database is +// present on the host. Uses the package-level cache in GPUs() so the +// call is free after the first invocation. +func ghwCards() []*gpu.GraphicsCard { + cards, err := GPUs() + if err != nil { + return nil + } + return cards +} + +// hasGPUVendor reports whether a GPU of the given vendor is present, +// consulting ghw first and then sysfs. The sysfs leg matters because it +// is the only one that works on images with no pci.ids database, where +// ghw enumeration fails outright. +func hasGPUVendor(vendor string) bool { + if ghwHasVendor(ghwCards(), vendor) { + return true + } + for _, g := range scanSysfsGPUs(defaultSysfsDRMPath) { + if g.Vendor == vendor { return true } } diff --git a/pkg/xsysinfo/gpuvendor_internal_test.go b/pkg/xsysinfo/gpuvendor_internal_test.go new file mode 100644 index 000000000..f24012916 --- /dev/null +++ b/pkg/xsysinfo/gpuvendor_internal_test.go @@ -0,0 +1,56 @@ +package xsysinfo + +import ( + "github.com/jaypipes/ghw/pkg/gpu" + "github.com/jaypipes/ghw/pkg/pci" + "github.com/jaypipes/pcidb" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func card(vendorID, vendorName string) *gpu.GraphicsCard { + return &gpu.GraphicsCard{ + DeviceInfo: &pci.Device{ + Vendor: &pcidb.Vendor{ID: vendorID, Name: vendorName}, + }, + } +} + +var _ = Describe("vendorFromGHW", func() { + It("identifies the vendor from the numeric PCI ID", func() { + Expect(vendorFromGHW([]*gpu.GraphicsCard{card("10de", "NVIDIA Corporation")})).To(Equal(VendorNVIDIA)) + Expect(vendorFromGHW([]*gpu.GraphicsCard{card("1002", "Advanced Micro Devices, Inc. [AMD/ATI]")})).To(Equal(VendorAMD)) + Expect(vendorFromGHW([]*gpu.GraphicsCard{card("8086", "Intel Corporation")})).To(Equal(VendorIntel)) + }) + + It("identifies the vendor even when it is missing from the pci.ids database", func() { + // ghw reads the vendor ID from the kernel modalias and only the + // name from pci.ids, so a card absent from an outdated database + // still carries a usable ID. + Expect(vendorFromGHW([]*gpu.GraphicsCard{card("8086", "unknown")})).To(Equal(VendorIntel)) + }) + + It("falls back to the vendor name when no usable ID is present", func() { + Expect(vendorFromGHW([]*gpu.GraphicsCard{card("", "NVIDIA Corporation")})).To(Equal(VendorNVIDIA)) + }) + + It("prefers a discrete NVIDIA GPU over an integrated Intel one regardless of enumeration order", func() { + cards := []*gpu.GraphicsCard{card("8086", "Intel Corporation"), card("10de", "NVIDIA Corporation")} + Expect(vendorFromGHW(cards)).To(Equal(VendorNVIDIA)) + }) + + It("prefers AMD over Intel", func() { + cards := []*gpu.GraphicsCard{card("8086", "Intel Corporation"), card("1002", "AMD/ATI")} + Expect(vendorFromGHW(cards)).To(Equal(VendorAMD)) + }) + + It("returns empty for a device that is not a known GPU vendor", func() { + // QEMU virtual VGA (1234:1111) and ASPEED BMC adapters land here. + Expect(vendorFromGHW([]*gpu.GraphicsCard{card("1234", "unknown")})).To(BeEmpty()) + }) + + It("tolerates cards with no device information at all", func() { + Expect(vendorFromGHW([]*gpu.GraphicsCard{{}, nil})).To(BeEmpty()) + Expect(vendorFromGHW(nil)).To(BeEmpty()) + }) +}) diff --git a/pkg/xsysinfo/hasgpu_internal_test.go b/pkg/xsysinfo/hasgpu_internal_test.go new file mode 100644 index 000000000..91b08d33a --- /dev/null +++ b/pkg/xsysinfo/hasgpu_internal_test.go @@ -0,0 +1,39 @@ +package xsysinfo + +import ( + "github.com/jaypipes/ghw/pkg/gpu" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ghwHasVendor", func() { + It("matches on the numeric PCI vendor ID", func() { + cards := []*gpu.GraphicsCard{card("8086", "Intel Corporation")} + Expect(ghwHasVendor(cards, VendorIntel)).To(BeTrue()) + Expect(ghwHasVendor(cards, VendorNVIDIA)).To(BeFalse()) + }) + + It("reports every vendor present, not only the highest-priority one", func() { + // A hybrid-graphics host has both. Asking "is there an Intel GPU" + // must not be answered by the discrete NVIDIA card outranking it. + cards := []*gpu.GraphicsCard{card("8086", "Intel Corporation"), card("10de", "NVIDIA Corporation")} + Expect(ghwHasVendor(cards, VendorNVIDIA)).To(BeTrue()) + Expect(ghwHasVendor(cards, VendorIntel)).To(BeTrue()) + Expect(ghwHasVendor(cards, VendorAMD)).To(BeFalse()) + }) + + It("matches the vendor name case-insensitively when no ID is available", func() { + // pci.ids spells it "NVIDIA Corporation"; callers pass the + // lowercase vendor constant. + Expect(ghwHasVendor([]*gpu.GraphicsCard{card("", "NVIDIA Corporation")}, VendorNVIDIA)).To(BeTrue()) + }) + + It("does not match a device that is not a known GPU vendor", func() { + Expect(ghwHasVendor([]*gpu.GraphicsCard{card("1234", "unknown")}, VendorIntel)).To(BeFalse()) + }) + + It("tolerates missing device information", func() { + Expect(ghwHasVendor([]*gpu.GraphicsCard{{}, nil}, VendorIntel)).To(BeFalse()) + Expect(ghwHasVendor(nil, VendorIntel)).To(BeFalse()) + }) +}) diff --git a/pkg/xsysinfo/sysfsgpu.go b/pkg/xsysinfo/sysfsgpu.go new file mode 100644 index 000000000..e32686a2a --- /dev/null +++ b/pkg/xsysinfo/sysfsgpu.go @@ -0,0 +1,202 @@ +package xsysinfo + +import ( + "github.com/jaypipes/ghw/pkg/gpu" + "os" + "path/filepath" + "strconv" + "strings" +) + +// defaultSysfsDRMPath is where the kernel exposes the DRM subsystem. +const defaultSysfsDRMPath = "/sys/class/drm" + +// pciVendorIDs maps PCI vendor IDs to the vendor constants used +// throughout this package. AMD graphics devices carry the ATI ID +// (0x1002), not AMD's host-bridge ID. +var pciVendorIDs = map[uint64]string{ + 0x10de: VendorNVIDIA, + 0x1002: VendorAMD, + 0x8086: VendorIntel, +} + +// vendorPriority is the order DetectGPUVendor reports a vendor in when +// a host has cards from more than one: a discrete accelerator always +// outranks an integrated display adapter. +var vendorPriority = []string{VendorNVIDIA, VendorAMD, VendorIntel} + +// sysfsGPU is a GPU discovered by walking the kernel's DRM sysfs tree. +type sysfsGPU struct { + Card string + Vendor string +} + +// scanSysfsGPUs enumerates GPUs straight from the kernel's DRM sysfs +// tree, identifying each by its raw PCI vendor ID. +// +// This is deliberately dependency-free. ghw, our richer source of GPU +// information, needs a pci.ids database file on disk to translate +// vendor IDs into names, and ghw.GPU() fails outright when it can't +// find one. Container images that ship no pci.ids (the Intel oneAPI +// image among them) therefore get no GPU detection at all, which is +// what issue #10941 reported: a correctly passed-through Arc A310 +// reported as "No GPU detected" with zero VRAM. Reading the vendor ID +// ourselves needs nothing but /sys. +func scanSysfsGPUs(root string) []sysfsGPU { + entries, err := os.ReadDir(root) + if err != nil { + return nil + } + var gpus []sysfsGPU + for _, entry := range entries { + name := entry.Name() + if !strings.HasPrefix(name, "card") { + continue + } + // cardN-- entries are display connectors hanging + // off a card, not cards in their own right. + if strings.ContainsRune(name, '-') { + continue + } + deviceDir := filepath.Join(root, name, "device") + vendorID, ok := readSysfsHexUint(filepath.Join(deviceDir, "vendor")) + if !ok { + continue + } + vendor, known := pciVendorIDs[vendorID] + if !known { + // Server BMC display adapters (ASPEED, Matrox) and virtual + // VGA devices land here. They're not usable accelerators. + continue + } + gpus = append(gpus, sysfsGPU{Card: name, Vendor: vendor}) + } + return gpus +} + +// sysfsVendorPriority returns the highest-priority GPU vendor found in +// the DRM tree, or "" when the host has no recognised GPU. +func sysfsVendorPriority(root string) string { + gpus := scanSysfsGPUs(root) + for _, want := range vendorPriority { + for _, g := range gpus { + if g.Vendor == want { + return want + } + } + } + return "" +} + +func readSysfsHexUint(path string) (uint64, bool) { + // #nosec G304 -- path is the DRM root (a package constant in production, + // a temp dir under test) joined with a ReadDir entry name and a fixed + // attribute filename; no external input reaches it. gosec suggests + // os.Root, which cannot work here: /sys/class/drm/cardN symlinks out to + // the PCI device tree, and os.Root rejects that as "path escapes from + // parent". + raw, err := os.ReadFile(path) + if err != nil { + return 0, false + } + return parseHexUint(string(raw)) +} + +// parseHexUint reads a hex PCI ID as written by either source: sysfs +// spells it "0x8086\n", ghw's modalias parse yields a bare "8086". +func parseHexUint(raw string) (uint64, bool) { + text := strings.TrimPrefix(strings.ToLower(strings.TrimSpace(raw)), "0x") + value, err := strconv.ParseUint(text, 16, 64) + if err != nil { + return 0, false + } + return value, true +} + +// vendorFromNames maps a set of PCI vendor name strings (as ghw +// resolves them from pci.ids) onto a vendor constant, applying +// vendorPriority so enumeration order can't decide the answer. +func vendorFromNames(names []string) string { + for _, want := range vendorPriority { + for _, name := range names { + if nameMatchesVendor(name, want) { + return want + } + } + } + return "" +} + +// nameMatchesVendor reports whether a PCI vendor name belongs to the +// requested vendor. The comparison is case-insensitive: pci.ids spells +// the vendor "NVIDIA Corporation" while callers pass the lowercase +// vendor constants. +func nameMatchesVendor(name, vendor string) bool { + return strings.Contains(strings.ToUpper(name), strings.ToUpper(vendor)) +} + +// vendorFromGHW resolves the vendor of the highest-priority card ghw +// enumerated. +// +// It prefers the numeric PCI vendor ID over the pci.ids vendor name: +// ghw reads the ID from the kernel's modalias and only the name from +// the database, so a card absent from an outdated pci.ids still carries +// a usable ID while its name reads "unknown". The name is kept as a +// fallback for devices that expose no parseable ID. +func vendorFromGHW(cards []*gpu.GraphicsCard) string { + var ids, names []string + for _, c := range cards { + if c == nil || c.DeviceInfo == nil || c.DeviceInfo.Vendor == nil { + continue + } + ids = append(ids, c.DeviceInfo.Vendor.ID) + names = append(names, c.DeviceInfo.Vendor.Name) + } + if vendor := vendorFromPCIIDs(ids); vendor != "" { + return vendor + } + return vendorFromNames(names) +} + +// vendorFromPCIIDs maps hex PCI vendor IDs onto a vendor constant, +// applying vendorPriority so enumeration order can't decide the answer. +func vendorFromPCIIDs(ids []string) string { + found := map[string]bool{} + for _, raw := range ids { + if id, ok := parseHexUint(raw); ok { + if vendor, known := pciVendorIDs[id]; known { + found[vendor] = true + } + } + } + for _, want := range vendorPriority { + if found[want] { + return want + } + } + return "" +} + +// ghwHasVendor reports whether any card ghw enumerated belongs to the +// given vendor. Unlike vendorFromGHW this is not a priority pick: a +// hybrid-graphics host must answer yes for both its integrated and its +// discrete GPU. +func ghwHasVendor(cards []*gpu.GraphicsCard, vendor string) bool { + for _, c := range cards { + if c == nil || c.DeviceInfo == nil || c.DeviceInfo.Vendor == nil { + continue + } + if id, ok := parseHexUint(c.DeviceInfo.Vendor.ID); ok { + if pciVendorIDs[id] == vendor { + return true + } + // A parseable ID that maps elsewhere is authoritative; don't + // let the name fall through and contradict it. + continue + } + if nameMatchesVendor(c.DeviceInfo.Vendor.Name, vendor) { + return true + } + } + return false +} diff --git a/pkg/xsysinfo/sysfsgpu_internal_test.go b/pkg/xsysinfo/sysfsgpu_internal_test.go new file mode 100644 index 000000000..35cece672 --- /dev/null +++ b/pkg/xsysinfo/sysfsgpu_internal_test.go @@ -0,0 +1,105 @@ +package xsysinfo + +import ( + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// writeSysfsCard lays out a fake /sys/class/drm entry the way the kernel +// does: card/device is a symlink into the PCI device tree, named after +// the card's BDF. +func writeSysfsCard(root, card, bdf string, attrs map[string]string) { + pciDir := filepath.Join(root, "..", "devices", "pci", bdf) + ExpectWithOffset(1, os.MkdirAll(pciDir, 0o755)).To(Succeed()) + for name, value := range attrs { + ExpectWithOffset(1, os.WriteFile(filepath.Join(pciDir, name), []byte(value), 0o644)).To(Succeed()) + } + cardDir := filepath.Join(root, card) + ExpectWithOffset(1, os.MkdirAll(cardDir, 0o755)).To(Succeed()) + ExpectWithOffset(1, os.Symlink(pciDir, filepath.Join(cardDir, "device"))).To(Succeed()) +} + +var _ = Describe("sysfs GPU discovery", func() { + var root string + + BeforeEach(func() { + root = filepath.Join(GinkgoT().TempDir(), "class", "drm") + Expect(os.MkdirAll(root, 0o755)).To(Succeed()) + }) + + Describe("scanSysfsGPUs", func() { + It("identifies an Intel discrete GPU by its PCI vendor ID", func() { + // Arc A310 (8086:56a6) as reported in issue #10941. + writeSysfsCard(root, "card1", "0000:03:00.0", map[string]string{ + "vendor": "0x8086\n", + }) + + gpus := scanSysfsGPUs(root) + + Expect(gpus).To(HaveLen(1)) + Expect(gpus[0].Vendor).To(Equal(VendorIntel)) + Expect(gpus[0].Card).To(Equal("card1")) + }) + + It("identifies NVIDIA and AMD GPUs by their PCI vendor IDs", func() { + writeSysfsCard(root, "card0", "0000:01:00.0", map[string]string{"vendor": "0x10de\n"}) + writeSysfsCard(root, "card1", "0000:02:00.0", map[string]string{"vendor": "0x1002\n"}) + + gpus := scanSysfsGPUs(root) + + Expect(gpus).To(HaveLen(2)) + Expect(gpus[0].Vendor).To(Equal(VendorNVIDIA)) + Expect(gpus[1].Vendor).To(Equal(VendorAMD)) + }) + + It("skips display connectors and cards with an unrecognised vendor", func() { + // An ASPEED BMC display adapter sits alongside the Arc on the + // machine in #10941; it must not be reported as a GPU. + writeSysfsCard(root, "card0", "0000:04:00.0", map[string]string{"vendor": "0x1a03\n"}) + writeSysfsCard(root, "card1", "0000:03:00.0", map[string]string{"vendor": "0x8086\n"}) + writeSysfsCard(root, "card1-DP-1", "0000:03:00.1", map[string]string{"vendor": "0x8086\n"}) + + gpus := scanSysfsGPUs(root) + + Expect(gpus).To(HaveLen(1)) + Expect(gpus[0].Card).To(Equal("card1")) + }) + + It("returns nothing when /sys/class/drm is absent", func() { + Expect(scanSysfsGPUs(filepath.Join(root, "nonexistent"))).To(BeEmpty()) + }) + }) + + Describe("sysfsVendorPriority", func() { + It("detects Intel when it is the only GPU present", func() { + writeSysfsCard(root, "card0", "0000:04:00.0", map[string]string{"vendor": "0x1a03\n"}) + writeSysfsCard(root, "card1", "0000:03:00.0", map[string]string{"vendor": "0x8086\n"}) + + Expect(sysfsVendorPriority(root)).To(Equal(VendorIntel)) + }) + + It("prefers a discrete NVIDIA GPU over an Intel integrated one", func() { + writeSysfsCard(root, "card0", "0000:00:02.0", map[string]string{"vendor": "0x8086\n"}) + writeSysfsCard(root, "card1", "0000:01:00.0", map[string]string{"vendor": "0x10de\n"}) + + Expect(sysfsVendorPriority(root)).To(Equal(VendorNVIDIA)) + }) + + It("prefers AMD over Intel", func() { + writeSysfsCard(root, "card0", "0000:00:02.0", map[string]string{"vendor": "0x8086\n"}) + writeSysfsCard(root, "card1", "0000:01:00.0", map[string]string{"vendor": "0x1002\n"}) + + Expect(sysfsVendorPriority(root)).To(Equal(VendorAMD)) + }) + + It("returns empty when no known GPU vendor is present", func() { + writeSysfsCard(root, "card0", "0000:04:00.0", map[string]string{"vendor": "0x1a03\n"}) + + Expect(sysfsVendorPriority(root)).To(BeEmpty()) + }) + }) + +})