From f0af2cb67b00c4e65ee6424e4f99fc9e7768e24b Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 26 Oct 2026 10:01:04 +0000 Subject: [PATCH 1/2] fix(examine): classify AMD GPUs by device id and one name table, and never downgrade an APU verdict What main still got wrong about integrated GPUs after #648 and #676, which fixed gfx90c, gfx1033/1035/1036 and the Windows PNP id: - The Linux PCI scan ignored the `[1002:xxxx]` device id `lspci -nn` prints on every line, so a GPU `pci.ids` cannot name ("Device") got no target and the "not an APU" default. It now asks the device-id table first, as the Windows probe asks the PNP id. - `examine` kept its own name table beside host_gpu's. It called Krackan Point gfx1150 (the device id and the 860M/840M it sells as say gfx1152, a separate TheRock family), and a codename list (`APU_KEYWORDS`) that answered "APU" without ever naming a target, so a Renoir iGPU came back with an empty `gfx_target`. Both are gone: `examine` asks host_gpu's table, which gains the codenames `lspci` prints (Strix Halo, Ryzen AI Max, Strix Point, Krackan, Phoenix1-4, Hawk Point, Rembrandt, Raphael, Mendocino, VanGogh, Renoir, Cezanne, Lucienne, Barcelo). The old list's "radeon 8045s" was never a product: the Strix Halo SKU names are 8060S, 8050S and 8040S (the Ryzen AI Max PRO 380's), which is what the shared table already carries. - Raven Ridge (gfx902), Picasso / Raven2 (gfx909) and Mendocino (gfx1037) were classified as discrete. No lookup table produces those targets; they arrive from KFD's packed gfx_target_version. - A `rocminfo` agent or `hipInfo` line overwrote `is_apu` with `gfx_is_apu_family`, which answers `false` both for a discrete card and for a target it has never heard of. `gfx_apu_verdict` tells the two apart (`DISCRETE_GFX_TARGETS` is its "no" half), and an unclassified target now leaves the earlier verdict standing. The agent-with-no-PCI-row branch falls back to the agent's marketing name, then to unset. This is latent while #393 makes the parser drop every agent on a real host. gfx90c keeps #648's decision that it has no package family. Because the name table can now answer gfx90c by codename, the install-side Windows display decoder and `preferred_amd_display` rank a name-identified gfx90c below every other recognised adapter, as they already rank a PCI-identified one, so a Vega iGPU listed first cannot hide a discrete card. An AMD PCI row nothing identifies is still reported as discrete, on purpose: reporting it as unknown would clear `has_discrete_amd` beside every discrete card the tables do not name (RX 5000, Vega, Radeon VII), on the hybrid host `fix-9-igpu-dgpu` exists for. The trade is documented on `classify_amd_marketing_name` and both sides are pinned through `diagnose()`. Tests: unit tests for each change, plus a property harness (examine_gpu_proptests.rs) over real `lspci` lines, `rocminfo` blocks, Windows rows and marketing names. The never-downgrade property draws agent targets nothing classifies a quarter of the time; on the device's own target an unconditional overwrite computes the same answer, so without them it could not fail. Every generator's measured reach has a floor. Each change was mutation-checked: restoring the old behaviour fails at least one test. Signed-off-by: Roman Inflianskas --- crates/rocm-core/src/examine.rs | 361 ++++- crates/rocm-core/src/examine_gpu_proptests.rs | 1301 +++++++++++++++++ crates/rocm-core/src/host_gpu.rs | 179 ++- xtask/src/ascii_output_contract.rs | 5 - 4 files changed, 1767 insertions(+), 79 deletions(-) create mode 100644 crates/rocm-core/src/examine_gpu_proptests.rs diff --git a/crates/rocm-core/src/examine.rs b/crates/rocm-core/src/examine.rs index d0fb7a020..f9137ebe0 100644 --- a/crates/rocm-core/src/examine.rs +++ b/crates/rocm-core/src/examine.rs @@ -57,22 +57,6 @@ const AMDGPU_INSTALL_MARKERS: &[&str] = &[ "/etc/yum.repos.d/rocm.repo", ]; -/// Marketing-name fragments that identify an AMD APU when `rocminfo` is absent. -const APU_KEYWORDS: &[&str] = &[ - "strix halo", - "ryzen ai max", - "phoenix", - "hawk point", - "strix point", - "krackan", - "rembrandt", - "raphael", - "barcelo", - "lucienne", - "renoir", - "cezanne", -]; - /// A single GPU as enumerated by `lspci`/`rocminfo` (Linux) or the display /// inventory (Windows). #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] @@ -1164,44 +1148,46 @@ fn probe_cpu_windows(e: &mut Examination) { // GPU probes // --------------------------------------------------------------------------- -/// Best-effort `(gfx_target, is_apu)` for an AMD marketing name. +/// Best-effort `(gfx_target, is_apu)` for an AMD marketing name or `lspci` +/// codename, or `("", false)` when the name resolves no part. +/// +/// The target comes from the crate's one name table, +/// [`crate::host_gpu::gfx_target_from_amd_marketing_name`], and the packaging +/// from classifying that target. `examine` used to keep a second, smaller copy +/// of that table here, which disagreed with it about Krackan Point (gfx1150 +/// here, gfx1152 there and in the device-id table), plus a list of codename +/// fragments that answered "APU?" without ever naming a target, so a Renoir +/// iGPU came back with an empty `gfx_target`. +/// +/// A name that resolves nothing answers `false`, "not an APU", and that is a +/// deliberate default for a PCI row rather than a fact. The only consumer that +/// acts on the verdict is `check_9_igpu_dgpu_collision`, which fires on +/// `has_apu && has_discrete_amd`: +/// +/// - reporting an unidentified *discrete* card as "cannot say" would clear +/// `has_discrete_amd` beside every card the tables do not name -- every RX +/// 5000, Vega 56/64 and Radeon VII -- on exactly the hybrid host that check +/// exists for; +/// - reporting an unidentified *integrated* GPU as discrete sets +/// `has_discrete_amd` on an iGPU-only host, where the check still cannot fire +/// because `has_apu` is false. The cost is one wrong field in `rocm examine +/// --json`, and the integrated set is small and named in the table, so an +/// unresolved row is far more often a discrete card. +/// +/// `rocminfo` and `hipInfo` revise this verdict when they report a target whose +/// packaging is known ([`gfx_apu_verdict`]); where they do not, it stands. fn classify_amd_marketing_name(name: &str) -> (String, bool) { - let mut n = name.to_lowercase(); - for deco in ["(tm)", "(r)", "(c)", "(\u{2122})"] { - n = n.replace(deco, " "); - } - let n = n.split_whitespace().collect::>().join(" "); - let contains = |needle: &str| n.contains(needle); - if contains("ryzen ai max") || contains("strix halo") { - return ("gfx1151".to_owned(), true); - } - if contains("radeon 8050s") || contains("radeon 8060s") || contains("radeon 8045s") { - return ("gfx1151".to_owned(), true); - } - if contains("radeon 880m") - || contains("radeon 890m") - || contains("strix point") - || contains("krackan") - { - return ("gfx1150".to_owned(), true); - } - if contains("radeon 780m") - || contains("radeon 760m") - || contains("radeon 740m") - || contains("phoenix") - || contains("hawk point") - { - return ("gfx1103".to_owned(), true); - } - (String::new(), APU_KEYWORDS.iter().any(|kw| n.contains(kw))) + let gfx = crate::host_gpu::gfx_target_from_amd_marketing_name(name).unwrap_or_default(); + (gfx.to_owned(), gfx_is_apu_family(gfx)) } /// Whether a gfx target belongs to an AMD APU family. /// -/// APUs: gfx90c (Vega iGPU in Renoir / Lucienne / Cezanne / Barcelo), the -/// RDNA2 iGPUs gfx1033 / gfx1035 / gfx1036 (Van Gogh, Rembrandt / Yellow Carp, -/// Raphael), gfx1103 (Phoenix / Hawk Point) and the gfx115x parts (Strix Point / -/// Strix Halo). +/// APUs: the Vega iGPUs gfx902 / gfx909 (Raven Ridge, Picasso / Raven2) and +/// gfx90c (Renoir / Lucienne / Cezanne / Barcelo), the RDNA2 iGPUs gfx1033 / +/// gfx1035 / gfx1036 / gfx1037 (Van Gogh, Rembrandt / Yellow Carp, Raphael, +/// Mendocino), gfx1103 (Phoenix / Hawk Point) and the gfx115x parts (Strix Point +/// / Strix Halo / Krackan Point). /// /// Their neighbours on both prefixes are *discrete* parts and must not match: /// gfx1100 / gfx1101 / gfx1102 (Navi 31 / 32 / 33) on gfx110x, and gfx1030 / @@ -1231,10 +1217,13 @@ fn classify_amd_marketing_name(name: &str) -> (String, bool) { /// GPU on the host is integrated. pub fn gfx_is_apu_family(gfx: &str) -> bool { let g = gfx.to_lowercase(); - // gfx90c: the Vega iGPU in Renoir / Lucienne / Cezanne / Barcelo. Compared - // whole (up to a feature suffix) so other gfx90x data-center parts such as - // gfx90a stay discrete. - if g == "gfx90c" || g.starts_with("gfx90c:") { + // The Vega iGPUs: gfx90c in Renoir / Lucienne / Cezanne / Barcelo, and + // gfx902 / gfx909 in Raven Ridge and Picasso / Raven2. Compared whole (up to + // a feature suffix) so the gfx90x data-center parts beside them -- gfx900, + // gfx906, gfx908, gfx90a -- stay discrete. No lookup table produces gfx902 + // or gfx909: they reach the CLI from KFD's packed `gfx_target_version` + // (90002, 90009), through `gfx_target_from_gc_version`. + if matches!(g.split(':').next(), Some("gfx90c" | "gfx902" | "gfx909")) { return true; } // gfx115x: every Strix part is an APU. @@ -1247,11 +1236,12 @@ pub fn gfx_is_apu_family(gfx: &str) -> bool { } // gfx103x: non-contiguous, unlike gfx110x's threshold. gfx1033/1035/1036 // are the RDNA2 APUs (Van Gogh variant aside, host_gpu's own PCI-id table - // maps them as such); gfx1030/1031/1032/1034 are Navi 21-24 desktop/mobile - // discrete cards (RX 6000-series, RX 6500 XT). A plain threshold would get - // this family wrong in both directions. + // maps them as such), and gfx1037 is Mendocino (Ryzen 7020), which KFD and + // DRM ip-discovery report as GC 10.3.7; gfx1030/1031/1032/1034 are Navi + // 21-24 desktop/mobile discrete cards (RX 6000-series, RX 6500 XT). A plain + // threshold would get this family wrong in both directions. if let Some(digit) = gfx_model_digit(&g, "gfx103") { - return matches!(digit, 3 | 5 | 6); + return matches!(digit, 3 | 5 | 6 | 7); } false } @@ -1264,6 +1254,56 @@ fn gfx_model_digit(gfx: &str, prefix: &str) -> Option { gfx.strip_prefix(prefix)?.chars().next()?.to_digit(10) } +/// Discrete gfx targets: every one a card or accelerator with memory of its own. +/// +/// This is the "no" half of [`gfx_apu_verdict`], which needs one because +/// [`gfx_is_apu_family`] answers `false` both for a discrete part and for a +/// target nothing here has heard of. It lists the discrete targets the lookup +/// tables can produce plus the ones the CLI installs for; +/// `every_target_a_lookup_table_produces_has_an_apu_verdict` fails when a table +/// gains a target that neither this list nor [`gfx_is_apu_family`] classifies. +/// +/// gfx942 covers both the MI300X accelerator and the MI300A, which is an APU +/// with unified HBM. It is listed discrete because MI300X is what ROCm hosts +/// run and what the CLI has always answered; telling the two apart needs a +/// signal the target does not carry. +const DISCRETE_GFX_TARGETS: &[&str] = &[ + // GCN / CDNA. + "gfx900", "gfx906", "gfx908", "gfx90a", "gfx942", "gfx950", + // RDNA1: Navi 10 / 12 / 14. + "gfx1010", "gfx1011", "gfx1012", // RDNA2: Navi 21 / 22 / 23 / 24. + "gfx1030", "gfx1031", "gfx1032", "gfx1034", // RDNA3: Navi 31 / 32 / 33. + "gfx1100", "gfx1101", "gfx1102", // RDNA4: Navi 44 / 48. + "gfx1200", "gfx1201", // The gfx125X-dcgpu family. + "gfx1250", +]; + +/// Whether a gfx target is an APU, or `None` when this crate does not know. +/// +/// The three-way answer is the point. A probe that folds a target into a +/// verdict another probe already reached needs to tell "this is a discrete +/// card" from "never heard of this target": the second is an absence of +/// evidence, and must leave the earlier verdict standing rather than replace +/// it with [`gfx_is_apu_family`]'s default `false`. Reading `rocminfo` or +/// `hipInfo` must not make the report worse than not reading it. +/// +/// A feature suffix (`gfx1036:xnack-`), a generic-target suffix +/// (`gfx1036-generic`) and case do not change the answer. +fn gfx_apu_verdict(gfx: &str) -> Option { + if gfx_is_apu_family(gfx) { + return Some(true); + } + let base: String = gfx + .trim() + .to_ascii_lowercase() + .chars() + .take_while(char::is_ascii_alphanumeric) + .collect(); + DISCRETE_GFX_TARGETS + .contains(&base.as_str()) + .then_some(false) +} + /// Whether an `lspci -nn` line describes a GPU this probe should enumerate. /// /// Instinct parts report PCI class `1200` ("Processing accelerators"), not a @@ -1293,6 +1333,17 @@ fn probe_gpus_lspci(e: &mut Examination) { .push("lspci returned non-zero; PCI enumeration incomplete".to_owned()); return; } + apply_lspci_gpus(e, &out); +} + +/// Fold an `lspci -nn -D` listing into the GPU list, against caller-supplied +/// output. +/// +/// Split from the process launch for the same reason +/// [`apply_rocminfo_gpu_agents`] is: what matters is how a row is classified, +/// and a test that had to run the real `lspci` could only assert it on the +/// hardware it happened to run on. +fn apply_lspci_gpus(e: &mut Examination, out: &str) { for line in out.lines() { if !is_lspci_gpu_line(line) { continue; @@ -1321,7 +1372,17 @@ fn probe_gpus_lspci(e: &mut Examination) { if !is_amd { continue; } - let (gfx_guess, is_apu_guess) = classify_amd_marketing_name(&name); + // `-nn` prints the PCI device id on the line, so ask it first, for the + // reason the Windows probe asks the PNP id first: the id names the + // ASIC, the marketing string only the brand. It matters most where the + // name says nothing at all -- for an id `pci.ids` does not know, `lspci` + // prints the bare word "Device". + let (gfx_guess, is_apu_guess) = match extract_lspci_device_id(line) + .and_then(|device_id| crate::host_gpu::gfx_target_from_amd_pci_device_id(&device_id)) + { + Some(target) => (target.to_owned(), gfx_is_apu_family(target)), + None => classify_amd_marketing_name(&name), + }; e.gpus.push(Gpu { name, gfx_target: gfx_guess, @@ -1332,6 +1393,24 @@ fn probe_gpus_lspci(e: &mut Examination) { } } +/// The AMD PCI device id an `lspci -nn` line carries: the four hex digits after +/// `1002:` in its `[vendor:device]` tag. +/// +/// Only AMD's vendor id is accepted. The tag is what tells a device id apart +/// from the other bracketed numbers on the line -- the PCI class (`[0300]`) and +/// any subsystem text `pci.ids` supplies -- and a number lifted from one of +/// those would decode to an unrelated part. +fn extract_lspci_device_id(line: &str) -> Option { + let lower = line.to_ascii_lowercase(); + let start = lower.rfind("[1002:")? + "[1002:".len(); + let device_id: String = lower[start..] + .chars() + .take_while(char::is_ascii_hexdigit) + .take(5) + .collect(); + (device_id.len() == 4 && lower[start + 4..].starts_with(']')).then_some(device_id) +} + /// Pull the marketing name out of an `lspci -nn` line: the text between the /// controller-kind `]:` and the trailing `[vendor:device]`. fn extract_lspci_name(line: &str) -> String { @@ -1444,9 +1523,30 @@ fn apply_rocminfo_gpu_agents(e: &mut Examination, out: &str) { if !marketing.is_empty() && gpu_name_is_unknown(&gpu.name) { gpu.name = marketing; } - gpu.is_apu = Some(gfx_is_apu_family(&gfx)); + // The target names the silicon, so where its packaging is known it + // outranks the PCI scan's guess from a marketing string. A target + // nothing here classifies is not evidence of a discrete GPU, so it + // leaves the PCI scan's verdict standing: before this, an agent + // reporting an unlisted target turned an APU the PCI scan had + // identified into a discrete card, clearing `has_apu` and with it + // the iGPU+dGPU collision diagnosis. (Latent while #393 makes the + // parser drop every agent on a real host; live the day it is fixed.) + if let Some(is_apu) = gfx_apu_verdict(&gfx) { + gpu.is_apu = Some(is_apu); + } } else { - let is_apu = gfx_is_apu_family(&gfx); + // An agent with no PCI row to pair with -- the ordinary ROCm + // container, which ships `rocminfo` but not `pciutils`. Same + // precedence, over the evidence there is: the target when its + // packaging is known, else the agent's own marketing name when that + // resolves a part. When neither answers, `is_apu` stays unset rather + // than defaulting to `false`, which `summarise_gpu_categories` would + // read as "a discrete GPU" -- the conclusion + // `probe_gpus_sysfs_fallback` reaches for the same reason. + let is_apu = gfx_apu_verdict(&gfx).or_else(|| { + let (named_target, named_is_apu) = classify_amd_marketing_name(&marketing); + (!named_target.is_empty()).then_some(named_is_apu) + }); e.gpus.push(Gpu { name: if marketing.is_empty() { "AMD GPU".to_owned() @@ -1455,7 +1555,7 @@ fn apply_rocminfo_gpu_agents(e: &mut Examination, out: &str) { }, gfx_target: gfx, is_amd: true, - is_apu: Some(is_apu), + is_apu, ..Gpu::default() }); } @@ -3176,7 +3276,11 @@ fn fill_unidentified_gpus_from_hipinfo(gpus: &mut [Gpu], hipinfo_stdout: &str) { else { continue; }; - gpu.is_apu = Some(gfx_is_apu_family(&gfx)); + // Same rule as `apply_rocminfo_gpu_agents`: a target whose packaging is + // known outranks the display probe's verdict, an unknown one leaves it. + if let Some(is_apu) = gfx_apu_verdict(&gfx) { + gpu.is_apu = Some(is_apu); + } gpu.gfx_target = gfx; } } @@ -6054,6 +6158,133 @@ mod tests { assert!(e.has_discrete_amd); } + /// The Vega and RDNA2 APUs that reach the CLI only as a target synthesised + /// from a GC version -- KFD's packed `gfx_target_version` 90002, 90009 and + /// 100307 -- and that a prefix rule never reached: Raven Ridge, Picasso / + /// Raven2 and Mendocino. Their gfx90x and gfx103x neighbours stay discrete. + #[test] + fn raven_picasso_and_mendocino_are_apus() { + for apu in [ + "gfx902", + "gfx909", + "gfx1037", + "GFX902", + "gfx909:xnack-", + "gfx1037:xnack-", + ] { + assert!(gfx_is_apu_family(apu), "{apu}"); + } + for discrete in ["gfx900", "gfx906", "gfx908", "gfx90a", "gfx1034", "gfx1030"] { + assert!(!gfx_is_apu_family(discrete), "{discrete}"); + } + } + + /// "Not an APU" and "never heard of it" are different answers, and the + /// folds that revise a verdict need to tell them apart. + #[test] + fn an_unclassified_target_has_no_apu_verdict() { + assert_eq!(gfx_apu_verdict("gfx1036"), Some(true)); + assert_eq!(gfx_apu_verdict("gfx1036-generic"), Some(true)); + assert_eq!(gfx_apu_verdict("gfx1100"), Some(false)); + assert_eq!(gfx_apu_verdict("GFX942:sramecc+:xnack-"), Some(false)); + assert_eq!(gfx_apu_verdict("gfx1100-generic"), Some(false)); + assert_eq!(gfx_apu_verdict("gfx1170"), None); + assert_eq!(gfx_apu_verdict(""), None); + } + + /// The `lspci -nn` device id comes from AMD's vendor tag and nowhere else. + #[test] + fn the_lspci_device_id_is_read_from_the_amd_vendor_tag() { + assert_eq!( + extract_lspci_device_id( + "0000:c5:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Krackan Point [Radeon 860M] [1002:1114] (rev c1)" + ) + .as_deref(), + Some("1114") + ); + // The class `[0300]` is a four-digit number in brackets too, and an + // NVIDIA line carries no AMD tag at all. + assert_eq!( + extract_lspci_device_id( + "0000:01:00.0 VGA compatible controller [0300]: NVIDIA Corporation \ + GA102 [GeForce RTX 3090] [10de:2204] (rev a1)" + ), + None + ); + // A tag that is not exactly four hex digits is not a device id. + assert_eq!(extract_lspci_device_id("Device [1002:11141]"), None); + assert_eq!(extract_lspci_device_id("Device [1002:111]"), None); + assert_eq!( + extract_lspci_device_id("no bracketed vendor tag here"), + None + ); + } + + /// `examine` names a GPU from the crate's one name table, codenames + /// included, so `lspci`'s codename for an iGPU yields its target rather + /// than an "APU, target unknown" guess. + /// + /// Krackan Point is the case that used to be wrong rather than missing: + /// `examine`'s own table called it gfx1150, while the device-id table and + /// the 860M/840M it sells as say gfx1152, a different TheRock family. + #[test] + fn examine_names_a_gpu_by_its_codename_from_the_shared_table() { + for (name, target) in [ + ( + "Advanced Micro Devices, Inc. [AMD/ATI] Krackan Point", + "gfx1152", + ), + ("Advanced Micro Devices, Inc. [AMD/ATI] Renoir", "gfx90c"), + ( + "Advanced Micro Devices, Inc. [AMD/ATI] Cezanne [Radeon Vega Series]", + "gfx90c", + ), + ("Advanced Micro Devices, Inc. [AMD/ATI] Raphael", "gfx1036"), + ( + "Advanced Micro Devices, Inc. [AMD/ATI] Mendocino", + "gfx1037", + ), + ("Advanced Micro Devices, Inc. [AMD/ATI] Phoenix1", "gfx1103"), + ( + "Advanced Micro Devices, Inc. [AMD/ATI] VanGogh [AMD Custom GPU 0405]", + "gfx1033", + ), + ("AMD Radeon(TM) 8040S Graphics", "gfx1151"), + ] { + assert_eq!( + classify_amd_marketing_name(name), + (target.to_owned(), true), + "{name}" + ); + } + // Discrete names come from the same table, so they now carry a target. + assert_eq!( + classify_amd_marketing_name("AMD Radeon RX 7900 XTX"), + ("gfx1100".to_owned(), false) + ); + } + + /// The Linux PCI scan prefers the device id to the name, the way the + /// Windows probe prefers the PNP id. + /// + /// The two inputs disagree on purpose: device id `73a0` is Navi 21 + /// (gfx1030, discrete) while the name is a Strix Halo SKU. No such part + /// ships, which is why it is the input -- only a pair that cannot both be + /// right pins precedence rather than agreement. + #[test] + fn lspci_scan_prefers_the_device_id_to_the_name() { + let mut e = Examination::default(); + apply_lspci_gpus( + &mut e, + "0000:03:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Radeon 8060S [1002:73a0] (rev c1)", + ); + assert_eq!(e.gpus.len(), 1, "{:?}", e.gpus); + assert_eq!(e.gpus[0].gfx_target, "gfx1030", "{:?}", e.gpus); + assert_eq!(e.gpus[0].is_apu, Some(false), "{:?}", e.gpus); + } + #[test] fn marketing_name_maps_strix_halo() { assert_eq!( @@ -6411,3 +6642,9 @@ mod tests { #[cfg(test)] #[path = "examine_driver_proptests.rs"] mod driver_proptests; + +/// Property tests for how the GPU probes above enumerate and classify AMD GPUs; +/// a child module so it can reach the private folds and classifiers. +#[cfg(test)] +#[path = "examine_gpu_proptests.rs"] +mod gpu_proptests; diff --git a/crates/rocm-core/src/examine_gpu_proptests.rs b/crates/rocm-core/src/examine_gpu_proptests.rs new file mode 100644 index 000000000..8c461e280 --- /dev/null +++ b/crates/rocm-core/src/examine_gpu_proptests.rs @@ -0,0 +1,1301 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! Property-based tests for how `examine` enumerates and classifies AMD GPUs. +//! +//! The generators are built from *real* fixture shapes -- `lspci -nn -D` lines, +//! `rocminfo` agent blocks, `Win32_VideoController` rows and AMD marketing +//! names -- and then mutated (case, whitespace, decorations, dropped brackets, +//! truncation). A uniform-random string generator never reaches the branches +//! that matter, so every strategy starts from a corpus and perturbs it. +//! +//! Each property states an invariant the *report* must hold, so a failure names +//! a defect a user would see. `generator_reach_is_measured_and_sufficient` +//! asserts that the generators actually reach the regions the properties are +//! about, so the harness cannot quietly go vacuous. + +use super::{ + Examination, Gpu, GpuProbeSources, apply_lspci_gpus, apply_rocminfo_gpu_agents, + classify_amd_marketing_name, extract_lspci_device_id, extract_lspci_name, + fill_unidentified_gpus_from_hipinfo, gfx_apu_verdict, gfx_is_apu_family, is_lspci_gpu_line, + parse_gpus_windows_output, probe_gpus_after_lspci, summarise_gpu_categories, +}; +use proptest::prelude::*; +use proptest::strategy::ValueTree; +use proptest::test_runner::{Config, TestRunner}; + +/// Case count: `default`, unless `PROPTEST_CASES` (proptest's own knob, which +/// an explicit count would otherwise override) asks for a longer run. +fn cases(default: u32) -> u32 { + std::env::var("PROPTEST_CASES") + .ok() + .and_then(|value| value.parse().ok()) + .unwrap_or(default) +} + +// --------------------------------------------------------------------------- +// Corpora: real strings, from pci.ids, this repo's own fixtures and the +// hardware named in ROCm/rocm-cli#448 / #449. +// --------------------------------------------------------------------------- + +/// `(device_name_as_lspci_prints_it, pci_device_id, true_gfx_target, +/// is_really_an_apu)`. +/// +/// The names are the `pci.ids` device strings an `lspci -nn` line carries after +/// the `[AMD/ATI]` vendor tag. The last two fields are ground truth about the +/// silicon, not about what the code says, so one row is always a *coherent* +/// host: the gfx target really is the one that device reports. +const AMD_LSPCI_DEVICES: &[(&str, &str, &str, bool)] = &[ + // Discrete RDNA2 / RDNA3 / RDNA4. + ( + "Navi 31 [Radeon RX 7900 XT/7900 XTX/7900 GRE/7900M]", + "744c", + "gfx1100", + false, + ), + ( + "Navi 32 [Radeon RX 7700 XT / 7800 XT]", + "747e", + "gfx1101", + false, + ), + ( + "Navi 33 [Radeon RX 7600/7600 XT/7600M XT/7600S/7700S / PRO W7600]", + "7480", + "gfx1102", + false, + ), + ( + "Navi 21 [Radeon RX 6800/6800 XT / 6900 XT]", + "73bf", + "gfx1030", + false, + ), + ( + "Navi 22 [Radeon RX 6700 XT / 6800M / 6950 XT]", + "73df", + "gfx1031", + false, + ), + ( + "Navi 23 [Radeon RX 6600/6600 XT/6600M]", + "73ff", + "gfx1032", + false, + ), + ( + "Navi 24 [Radeon RX 6400/6500 XT/6500M]", + "743f", + "gfx1034", + false, + ), + ("Navi 48 [Radeon RX 9070/9070 XT]", "7550", "gfx1201", false), + // Datacenter. + ("Aqua Vanjaram [Instinct MI300X]", "74a1", "gfx942", false), + // APUs, oldest first. Every one is an integrated GPU sharing system + // memory; `has_apu` must be true on a host that has one. + ("Renoir", "1636", "gfx90c", true), + ("Cezanne", "1638", "gfx90c", true), + ("Lucienne", "164c", "gfx90c", true), + ("Barcelo", "15e7", "gfx90c", true), + ("VanGogh [AMD Custom GPU 0405]", "163f", "gfx1033", true), + ("Rembrandt [Radeon 680M]", "1681", "gfx1035", true), + ("Raphael", "164e", "gfx1036", true), + // No device-id row: only the codename identifies it. + ("Mendocino", "1506", "gfx1037", true), + ("Phoenix1", "15bf", "gfx1103", true), + ("Phoenix3", "1900", "gfx1103", true), + ("Strix [Radeon 880M / 890M]", "150e", "gfx1150", true), + ("Krackan Point [Radeon 860M]", "1114", "gfx1152", true), + // Strix Halo, as reported in ROCm/rocm-cli#449: pci.ids has no entry, so + // `lspci` prints the bare word "Device", and the device-id table has no + // row for 1586 either (#507). + ("Device", "1586", "gfx1151", true), +]; + +/// Corpus rows that nothing can identify: no `pci.ids` name, and an id the +/// device-id table does not map. They stay in the corpus for the parsing and +/// totality properties, and are excluded from the classification sweeps. +const UNIDENTIFIABLE_LSPCI_DEVICES: &[&str] = &["Device"]; + +/// PCI classes an `lspci -nn` line can carry: the ones the probe enumerates, +/// and last the bridge it must not. +const LSPCI_CLASSES: &[(&str, bool)] = &[ + ("VGA compatible controller [0300]", true), + ("Display controller [0380]", true), + ("3D controller [0302]", true), + ("Processing accelerators [1200]", true), + ("PCI bridge [0604]", false), +]; + +/// AMD marketing names as `rocminfo`'s `Marketing Name:` and Windows' +/// `Win32_VideoController.Name` report them, with ground truth about the part. +/// +/// `(name, true_gfx_target, is_really_an_apu, pci_device_id)`. The device id is +/// the one the part's Windows PNP id carries, where the device-id table maps +/// it; [`windows_corpus`] draws only those rows. +const AMD_MARKETING_NAMES: &[(&str, &str, bool, Option<&str>)] = &[ + ("AMD Radeon RX 7900 XTX", "gfx1100", false, None), + ("AMD Radeon RX 7800 XT", "gfx1101", false, None), + ("AMD Radeon RX 7600", "gfx1102", false, None), + ("AMD Radeon RX 9070 XT", "gfx1201", false, Some("7550")), + ("AMD Radeon PRO W7900", "gfx1100", false, None), + ("AMD Instinct MI300X", "gfx942", false, None), + ("AMD Radeon RX 6900 XT", "gfx1030", false, Some("73a0")), + ("AMD Radeon RX 6600M", "gfx1032", false, Some("73e0")), + ("AMD Radeon RX 6500M", "gfx1034", false, Some("743f")), + // APUs. + ( + "AMD Radeon(TM) 610M Graphics", + "gfx1036", + true, + Some("164e"), + ), + ( + "AMD Radeon(TM) 660M Graphics", + "gfx1035", + true, + Some("1681"), + ), + ( + "AMD Radeon(TM) 680M Graphics", + "gfx1035", + true, + Some("1681"), + ), + ( + "AMD Radeon(TM) 740M Graphics", + "gfx1103", + true, + Some("15bf"), + ), + ( + "AMD Radeon(TM) 760M Graphics", + "gfx1103", + true, + Some("15bf"), + ), + ( + "AMD Radeon(TM) 780M Graphics", + "gfx1103", + true, + Some("15bf"), + ), + ("AMD Radeon(TM) 820M Graphics", "gfx1153", true, None), + ( + "AMD Radeon(TM) 840M Graphics", + "gfx1152", + true, + Some("1114"), + ), + ( + "AMD Radeon(TM) 860M Graphics", + "gfx1152", + true, + Some("1114"), + ), + ("AMD Radeon(TM) 880M Graphics", "gfx1150", true, None), + ("AMD Radeon(TM) 890M Graphics", "gfx1150", true, None), + ("AMD Radeon 8040S Graphics", "gfx1151", true, None), + ("AMD Radeon 8050S Graphics", "gfx1151", true, None), + ("AMD Radeon 8060S Graphics", "gfx1151", true, None), + ( + "AMD RYZEN AI MAX+ 395 w/ Radeon 8060S", + "gfx1151", + true, + None, + ), + ("AMD Custom GPU 0405", "gfx1033", true, Some("163f")), +]; + +/// gfx targets a real host reports, with ground truth about packaging. +/// +/// gfx902, gfx909 and gfx1037 are here because no lookup table produces them: +/// they reach the CLI only through `gfx_target_from_gc_version`, from KFD's +/// packed `gfx_target_version` or DRM ip-discovery, so the table drift guard +/// cannot see them and ground truth has to. +const GFX_TARGETS: &[(&str, bool)] = &[ + ("gfx900", false), + ("gfx906", false), + ("gfx908", false), + ("gfx90a", false), + ("gfx942", false), + ("gfx950", false), + ("gfx1010", false), + ("gfx1011", false), + ("gfx1012", false), + ("gfx1030", false), + ("gfx1031", false), + ("gfx1032", false), + ("gfx1034", false), + ("gfx1100", false), + ("gfx1101", false), + ("gfx1102", false), + ("gfx1200", false), + ("gfx1201", false), + ("gfx1250", false), + // APU targets. + ("gfx902", true), + ("gfx909", true), + ("gfx90c", true), + ("gfx1033", true), + ("gfx1035", true), + ("gfx1036", true), + ("gfx1037", true), + ("gfx1103", true), + ("gfx1150", true), + ("gfx1151", true), + ("gfx1152", true), + ("gfx1153", true), +]; + +/// Well-formed gfx targets that nothing in this crate classifies: silicon newer +/// than the tables. A `rocminfo` agent reporting one of these is evidence of a +/// GPU, not of its packaging. +const UNCLASSIFIED_GFX_TARGETS: &[&str] = &["gfx1170", "gfx1180", "gfx1260", "gfx1300"]; + +// --------------------------------------------------------------------------- +// Mutators: the perturbations real-world messiness applies to these strings. +// --------------------------------------------------------------------------- + +/// Textual perturbations applied to a fixture-derived string. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Mutation { + None, + Upper, + Lower, + PadInnerWhitespace, + PadOuterWhitespace, + TrademarkDecoration, + RegisteredDecoration, + TruncateTail, + TruncateHead, + DropBrackets, + TabsForSpaces, +} + +fn mutation() -> impl Strategy { + prop_oneof![ + 6 => Just(Mutation::None), + 2 => Just(Mutation::Upper), + 2 => Just(Mutation::Lower), + 2 => Just(Mutation::PadInnerWhitespace), + 2 => Just(Mutation::PadOuterWhitespace), + 2 => Just(Mutation::TrademarkDecoration), + 1 => Just(Mutation::RegisteredDecoration), + 1 => Just(Mutation::TruncateTail), + 1 => Just(Mutation::TruncateHead), + 1 => Just(Mutation::DropBrackets), + 1 => Just(Mutation::TabsForSpaces), + ] +} + +/// The mutations that preserve the *identity* of the device being named. +/// +/// Case, whitespace and vendor decorations do; truncation and bracket removal +/// can destroy the token a lookup keys on. Properties that assert a +/// device-specific verdict draw from this narrower set rather than filtering +/// the wider one, which would spend proptest's reject budget on draws it was +/// always going to discard. +fn identity_preserving_mutation() -> impl Strategy { + prop_oneof![ + 6 => Just(Mutation::None), + 2 => Just(Mutation::Upper), + 2 => Just(Mutation::Lower), + 2 => Just(Mutation::PadInnerWhitespace), + 2 => Just(Mutation::PadOuterWhitespace), + 2 => Just(Mutation::TrademarkDecoration), + 1 => Just(Mutation::RegisteredDecoration), + 1 => Just(Mutation::TabsForSpaces), + ] +} + +fn apply_mutation(text: &str, m: Mutation) -> String { + match m { + Mutation::None => text.to_owned(), + Mutation::Upper => text.to_uppercase(), + Mutation::Lower => text.to_lowercase(), + Mutation::PadInnerWhitespace => text.replace(' ', " "), + Mutation::PadOuterWhitespace => format!(" {text}\t "), + Mutation::TrademarkDecoration => text.replacen("Radeon", "Radeon(TM)", 1), + Mutation::RegisteredDecoration => text.replacen("AMD", "AMD(R)", 1), + Mutation::TruncateTail => { + let keep = text.len().saturating_mul(2) / 3; + text.chars().take(keep).collect() + } + Mutation::TruncateHead => { + let drop = text.len() / 4; + text.chars().skip(drop).collect() + } + Mutation::DropBrackets => text.replace(['[', ']'], ""), + Mutation::TabsForSpaces => text.replace(' ', "\t"), + } +} + +// --------------------------------------------------------------------------- +// Strategies and fixtures +// --------------------------------------------------------------------------- + +/// A plausible PCI address, including the high bus numbers a Strix Halo iGPU +/// enumerates on (`0000:66:00.0` in #449). +fn pci_address() -> impl Strategy { + (0u32..2, 0u32..0x100, 0u32..0x20, 0u32..8) + .prop_map(|(dom, bus, dev, func)| format!("{dom:04x}:{bus:02x}:{dev:02x}.{func}")) +} + +/// A full `lspci -nn -D` line drawn from the real corpus, with the real vendor +/// tag, an optional revision suffix, and a mutation applied. +fn lspci_line() -> impl Strategy +{ + ( + pci_address(), + proptest::sample::select(LSPCI_CLASSES), + proptest::sample::select(AMD_LSPCI_DEVICES), + proptest::option::of(0u32..0x100), + mutation(), + ) + .prop_map(|(addr, class, device, rev, m)| { + let rev = rev.map_or_else(String::new, |r| format!(" (rev {r:02x})")); + let line = format!( + "{addr} {}: Advanced Micro Devices, Inc. [AMD/ATI] {} [1002:{}]{rev}", + class.0, device.0, device.1 + ); + (apply_mutation(&line, m), device) + }) +} + +/// A marketing name drawn from the real corpus with any mutation applied, +/// paired with the ground truth for the part it names. +fn marketing_name() -> impl Strategy< + Value = ( + String, + (&'static str, &'static str, bool, Option<&'static str>), + ), +> { + (proptest::sample::select(AMD_MARKETING_NAMES), mutation()) + .prop_map(|(entry, m)| (apply_mutation(entry.0, m), entry)) +} + +/// The same, restricted to spellings that still name the same device. +fn marketing_name_spelled_differently() -> impl Strategy< + Value = ( + String, + (&'static str, &'static str, bool, Option<&'static str>), + ), +> { + ( + proptest::sample::select(AMD_MARKETING_NAMES), + identity_preserving_mutation(), + ) + .prop_map(|(entry, m)| (apply_mutation(entry.0, m), entry)) +} + +/// Marketing names of parts that really are APUs, differently spelled. +fn apu_marketing_name() -> impl Strategy< + Value = ( + String, + (&'static str, &'static str, bool, Option<&'static str>), + ), +> { + let apus: Vec<_> = AMD_MARKETING_NAMES + .iter() + .copied() + .filter(|entry| entry.2) + .collect(); + assert!(!apus.is_empty(), "the marketing corpus has no APU"); + ( + proptest::sample::select(apus), + identity_preserving_mutation(), + ) + .prop_map(|(entry, m)| (apply_mutation(entry.0, m), entry)) +} + +/// `lspci` entries for parts that really are APUs and that the scan can +/// identify, by name or by device id. +fn apu_lspci_devices() -> Vec<(&'static str, &'static str, &'static str, bool)> { + let apus: Vec<_> = AMD_LSPCI_DEVICES + .iter() + .copied() + .filter(|device| device.3 && !UNIDENTIFIABLE_LSPCI_DEVICES.contains(&device.0)) + .collect(); + assert!(!apus.is_empty(), "the lspci corpus has no identifiable APU"); + apus +} + +/// The gfx target a `rocminfo` agent on an APU host reports: the device's own +/// target or, a quarter of the time, one nothing in this crate classifies. +/// +/// The unclassified half is the one that can tell the never-downgrade rule from +/// an unconditional overwrite. On the device's own target the two compute the +/// same answer, since every APU target in the corpus is classified, so a +/// property drawing only those could not fail with the rule removed. +fn agent_target_for(device_target: &'static str) -> impl Strategy { + prop_oneof![ + 3 => Just(device_target), + 1 => proptest::sample::select(UNCLASSIFIED_GFX_TARGETS), + ] +} + +/// The marketing-name rows whose PNP device id the device-id table maps. +/// +/// Asserted to stay large enough to matter, so the Windows property's corpus +/// cannot shrink to nothing without a test failing. +fn windows_corpus() -> Vec<(&'static str, &'static str, bool, &'static str)> { + let rows: Vec<_> = AMD_MARKETING_NAMES + .iter() + .filter_map(|(name, gfx, apu, id)| id.map(|id| (*name, *gfx, *apu, id))) + .collect(); + assert!( + rows.len() >= 10, + "only {} marketing-name rows carry a device id; the Windows property \ + would sample a handful of parts", + rows.len() + ); + rows +} + +/// What the real PCI scan makes of one corpus device. +/// +/// Every test that needs a PCI-sourced GPU goes through this rather than +/// assembling one from the classifiers: a test that re-implements the probe's +/// logic pins the re-implementation, and keeps passing when the probe stops +/// reading the device id. +fn pci_scanned(device: &str, device_id: &str, addr: &str) -> Gpu { + let line = format!( + "{addr} VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] {device} [1002:{device_id}] (rev c1)" + ); + let mut e = Examination::default(); + apply_lspci_gpus(&mut e, &line); + assert_eq!(e.gpus.len(), 1, "{line:?} must enumerate as one GPU"); + e.gpus.remove(0) +} + +/// A `Win32_VideoController` row as `WIN_GPU_SCRIPT` prints it: name, driver +/// version, PNP device id. +fn windows_row(name: &str, device_id: &str) -> String { + format!("{name}\t32.0.1\tPCI\\VEN_1002&DEV_{device_id}&SUBSYS_00001002&REV_C1") +} + +/// Every PCI device id the device-id table maps, with its target. +/// +/// The table is a `match`, not an iterable, so its whole input domain is swept. +fn every_mapped_pci_device_id() -> Vec<(String, &'static str)> { + let ids: Vec<_> = (0..=0xffff_u32) + .map(|id| format!("{id:04x}")) + .filter_map(|id| crate::host_gpu::gfx_target_from_amd_pci_device_id(&id).map(|t| (id, t))) + .collect(); + assert!(!ids.is_empty(), "the device-id table maps no id at all"); + ids +} + +/// A `rocminfo` agent listing for the given targets, in the agent block shape +/// the current parser can digest (no ISA sub-entries). +fn rocminfo_output(agents: &[(&str, &str)]) -> String { + rocminfo_output_shaped(agents, false) +} + +/// `with_isa_entries` reproduces what `rocminfo` since ROCm 5 prints: each GPU +/// agent is followed by indented ISA sub-entries that *also* begin with +/// `Name:`. The parser keeps a running `cur_name` and lets those lines +/// overwrite the agent's gfx name, so the agent is dropped and the fold does +/// nothing -- ROCm/rocm-cli#393. +fn rocminfo_output_shaped(agents: &[(&str, &str)], with_isa_entries: bool) -> String { + use std::fmt::Write as _; + let mut out = String::from( + "=====================\nHSA System Attributes\n=====================\n\ + Runtime Version: 1.1\n\n", + ); + out.push_str( + "==========\nHSA Agents\n==========\n*******\nAgent 1\n*******\n \ + Name: AMD Ryzen 9 7950X\n \ + Marketing Name: AMD Ryzen 9 7950X\n \ + Device Type: CPU\n", + ); + for (index, (gfx, marketing)) in agents.iter().enumerate() { + let _ = write!( + out, + "*******\nAgent {}\n*******\n Name: {gfx}\n \ + Marketing Name: {marketing}\n Device Type: GPU\n", + index + 2 + ); + if with_isa_entries { + let family = gfx.get(..5).unwrap_or(gfx); + let _ = write!( + out, + " ISA Info:\n ISA 1\n Name: \ + amdgcn-amd-amdhsa--{gfx}\n ISA 2\n Name: \ + amdgcn-amd-amdhsa--{family}-generic\n" + ); + } + } + out +} + +// --------------------------------------------------------------------------- +// Properties +// --------------------------------------------------------------------------- + +proptest! { + #![proptest_config(Config { + cases: cases(2048), + failure_persistence: None, + ..Config::default() + })] + + /// Totality: no fixture-derived or mutated `lspci` line may panic any of + /// the classifiers, or the PCI scan that composes them, and a single line + /// never enumerates as more than one GPU. + #[test] + fn classifiers_never_panic_on_a_mutated_lspci_line((line, _) in lspci_line()) { + let _ = is_lspci_gpu_line(&line); + let _ = extract_lspci_device_id(&line); + let (gfx, _) = classify_amd_marketing_name(&extract_lspci_name(&line)); + let _ = gfx_apu_verdict(&gfx); + let mut e = Examination::default(); + apply_lspci_gpus(&mut e, &line); + prop_assert!(e.gpus.len() <= 1, "{line:?} enumerated as {:?}", e.gpus); + } + + /// `extract_lspci_name` recovers exactly the vendor-plus-device text of an + /// unmutated `lspci -nn` line: everything between the class `]:` and the + /// trailing `[vendor:device]`. + #[test] + fn lspci_name_extraction_round_trips( + addr in pci_address(), + class in proptest::sample::select(LSPCI_CLASSES), + device in proptest::sample::select(AMD_LSPCI_DEVICES), + rev in proptest::option::of(0u32..0x100), + ) { + let rev = rev.map_or_else(String::new, |r| format!(" (rev {r:02x})")); + let line = format!( + "{addr} {}: Advanced Micro Devices, Inc. [AMD/ATI] {} [1002:{}]{rev}", + class.0, device.0, device.1 + ); + let expected = format!("Advanced Micro Devices, Inc. [AMD/ATI] {}", device.0); + prop_assert_eq!(extract_lspci_name(&line), expected); + prop_assert_eq!(extract_lspci_device_id(&line), Some(device.1.to_owned())); + } + + /// A name that names an APU must not be reported as "not an APU". + #[test] + fn examine_never_calls_a_known_apu_discrete((name, truth) in apu_marketing_name()) { + let (target, is_apu) = classify_amd_marketing_name(&name); + prop_assert!( + is_apu, + "{name} is an APU ({}) but examine says is_apu=false, gfx_target={target:?}", + truth.1, + ); + } + + /// Folding a `rocminfo` reading into the report never *downgrades* an APU + /// verdict the PCI scan already reached. More evidence must not produce a + /// worse answer -- including when the agent reports a target this crate + /// does not classify, which is what [`agent_target_for`] draws. + #[test] + fn rocminfo_never_downgrades_an_apu_verdict( + (device, agent_target) in proptest::sample::select(apu_lspci_devices()) + .prop_flat_map(|device| (Just(device), agent_target_for(device.2))), + addr in pci_address(), + marketing in proptest::sample::select(&["", "AMD Radeon Graphics"][..]), + ) { + let gpu = pci_scanned(device.0, device.1, &addr); + prop_assert!( + gpu.is_apu == Some(true), + "the PCI scan must already know {} is an APU before this property can \ + say anything about preserving that verdict: {:?}", + device.0, + gpu, + ); + + let mut e = Examination { + gpus: vec![gpu], + ..Examination::default() + }; + apply_rocminfo_gpu_agents(&mut e, &rocminfo_output(&[(agent_target, marketing)])); + summarise_gpu_categories(&mut e); + prop_assert_eq!(&e.gpus[0].gfx_target, agent_target, "the agent's target is recorded"); + prop_assert!( + e.has_apu, + "lspci classified {} as an APU, then rocminfo reporting {} flipped has_apu \ + to false: {:?}", + device.0, + agent_target, + e.gpus, + ); + prop_assert!( + !e.has_discrete_amd, + "an APU-only host must not report has_discrete_amd (agent {}): {:?}", + agent_target, + e.gpus, + ); + } + + /// Classification does not depend on spelling: case, inner/outer + /// whitespace and vendor decorations name the same device. + #[test] + fn classification_is_spelling_invariant((name, truth) in marketing_name_spelled_differently()) { + let canonical = classify_amd_marketing_name(truth.0); + let mutated = classify_amd_marketing_name(&name); + prop_assert_eq!( + &canonical, + &mutated, + "{:?} and {:?} are the same device but classify differently", + truth.0, + name, + ); + } + + /// Both APU answers agree with ground truth for every target a real host + /// reports, whatever its case or feature suffix: `gfx_is_apu_family` says + /// yes or no, and `gfx_apu_verdict` knows the answer. + #[test] + fn gfx_apu_verdicts_match_ground_truth( + gfx in proptest::sample::select(GFX_TARGETS), + suffix in proptest::sample::select(&["", ":xnack-", ":sramecc+:xnack-", ":sramecc-"][..]), + upper in any::(), + ) { + let spelled = if upper { + format!("{}{suffix}", gfx.0.to_uppercase()) + } else { + format!("{}{suffix}", gfx.0) + }; + prop_assert_eq!(gfx_is_apu_family(&spelled), gfx.1, "gfx_is_apu_family({})", spelled); + prop_assert_eq!(gfx_apu_verdict(&spelled), Some(gfx.1), "gfx_apu_verdict({})", spelled); + } + + /// `examine`'s Windows display probe and the install-side display decoder + /// read the same adapter to the same target. + /// + /// They share the device-id and name tables but not the wiring: `examine` + /// splits `namedriverpnp` and asks the PNP id first, the install + /// side reads `namepnp` rows and ranks a gfx90c iGPU below every other + /// adapter. A change to either wiring that reads the PNP id from the wrong + /// column, or prefers the name, makes them disagree here. + #[test] + fn examine_and_the_install_probe_read_a_windows_row_alike( + (name, gfx, apu, device_id) in proptest::sample::select(windows_corpus()), + ) { + let mut e = Examination::default(); + parse_gpus_windows_output(&windows_row(name, device_id), &mut e); + prop_assert_eq!(e.gpus.len(), 1); + let install = crate::host_gpu::parse_windows_display_gfx_target(&format!( + "{name}\tPCI\\VEN_1002&DEV_{device_id}&SUBSYS_00001002&REV_C1" + )); + prop_assert_eq!(install.as_deref(), Some(gfx), "install side, {}", name); + prop_assert_eq!(&e.gpus[0].gfx_target, gfx, "examine, {}", name); + prop_assert_eq!(e.gpus[0].is_apu, Some(apu), "examine, {}", name); + } +} + +// --------------------------------------------------------------------------- +// Exhaustive sweeps: the whole corpus, so a failure lists every case rather +// than the one minimal case a shrinker settles on. +// --------------------------------------------------------------------------- + +/// Every APU in the corpora is classified as one, by every door. +#[test] +fn every_known_apu_is_classified_as_an_apu() { + let mut misses: Vec = Vec::new(); + for (name, target, is_apu, _) in AMD_MARKETING_NAMES { + if !is_apu { + continue; + } + let (examine_target, examine_is_apu) = classify_amd_marketing_name(name); + if !examine_is_apu { + misses.push(format!( + " marketing name {name:?} ({target}): is_apu=false, gfx_target={examine_target:?}" + )); + } + } + for (gfx, is_apu) in GFX_TARGETS { + if *is_apu && !gfx_is_apu_family(gfx) { + misses.push(format!(" gfx target {gfx}: gfx_is_apu_family says false")); + } + } + for (device, id, _gfx, is_apu) in AMD_LSPCI_DEVICES { + if !is_apu || UNIDENTIFIABLE_LSPCI_DEVICES.contains(device) { + continue; + } + let gpu = pci_scanned(device, id, "0000:04:00.0"); + if gpu.is_apu != Some(true) { + misses.push(format!( + " lspci device {device:?} [1002:{id}]: the PCI scan says {gpu:?}" + )); + } + } + assert!( + misses.is_empty(), + "parts that are APUs but are not classified as APUs:\n{}", + misses.join("\n") + ); +} + +/// No discrete part in the corpora is classified as an APU -- the other +/// direction, which keeps a codename pattern from reaching past its part. +#[test] +fn no_discrete_part_is_classified_as_an_apu() { + let mut wrong: Vec = Vec::new(); + for (name, gfx, is_apu, _) in AMD_MARKETING_NAMES { + if !is_apu && classify_amd_marketing_name(name).1 { + wrong.push(format!(" marketing name {name:?} ({gfx})")); + } + } + for (device, id, gfx, is_apu) in AMD_LSPCI_DEVICES { + if !is_apu && pci_scanned(device, id, "0000:03:00.0").is_apu == Some(true) { + wrong.push(format!(" lspci {device:?} [1002:{id}] ({gfx})")); + } + } + assert!( + wrong.is_empty(), + "discrete parts classified as APUs:\n{}", + wrong.join("\n") + ); +} + +/// A reported `gfx_target` never *contradicts* the part: an unresolved lookup +/// is recoverable, a wrong answer selects the wrong runtime build. +#[test] +fn no_part_is_labelled_with_the_wrong_gfx_target() { + let mut wrong: Vec = Vec::new(); + for (device, id, gfx, _is_apu) in AMD_LSPCI_DEVICES { + let guess = pci_scanned(device, id, "0000:04:00.0").gfx_target; + if !guess.is_empty() && guess != *gfx { + wrong.push(format!( + " lspci {device:?} [1002:{id}] is {gfx}, but examine reports {guess}" + )); + } + } + for (name, gfx, _is_apu, _) in AMD_MARKETING_NAMES { + let guess = classify_amd_marketing_name(name).0; + if !guess.is_empty() && guess != *gfx { + wrong.push(format!( + " marketing name {name:?} is {gfx}, but examine reports {guess}" + )); + } + } + assert!( + wrong.is_empty(), + "parts labelled with a gfx target that is not theirs:\n{}", + wrong.join("\n") + ); +} + +/// Every gfx target a lookup table can hand back has an APU verdict. +/// +/// The name and device-id tables are keyed by name and id, and the packaging +/// knowledge by target, so nothing ties them together. A table entry for a +/// target neither `gfx_is_apu_family` nor `DISCRETE_GFX_TARGETS` knows would +/// answer "cannot say" for it, and the rocminfo fold would then leave a wrong +/// PCI-scan verdict standing. +#[test] +fn every_target_a_lookup_table_produces_has_an_apu_verdict() { + let mut unclassified: Vec = Vec::new(); + for entry in crate::host_gpu::AMD_MARKETING_GFX_TARGETS { + if gfx_apu_verdict(entry.gfx_target).is_none() { + unclassified.push(format!( + " marketing pattern {:?} -> {}", + entry.pattern, entry.gfx_target + )); + } + } + for (id, target) in every_mapped_pci_device_id() { + if gfx_apu_verdict(target).is_none() { + unclassified.push(format!(" PCI device id {id} -> {target}")); + } + } + unclassified.sort(); + unclassified.dedup(); + assert!( + unclassified.is_empty(), + "targets a lookup table produces but nothing classifies:\n{}", + unclassified.join("\n") + ); + for target in UNCLASSIFIED_GFX_TARGETS { + assert_eq!( + gfx_apu_verdict(target), + None, + "{target} is meant to be a target nothing classifies; the \ + never-downgrade property draws on that" + ); + } +} + +/// The Linux PCI scan classifies a row by its device id when the name carries +/// nothing. +/// +/// `lspci` prints the bare word `Device` for an id `pci.ids` does not know, so +/// on these rows only the `[1002:xxxx]` tag can identify the part. Every id the +/// device-id table maps is swept, so a scan that falls back to reading the name +/// alone fails here for all of them at once. +#[test] +fn an_lspci_row_is_classified_by_its_device_id_when_its_name_says_nothing() { + let mut wrong: Vec = Vec::new(); + for (id, target) in every_mapped_pci_device_id() { + let gpu = pci_scanned("Device", &id, "0000:66:00.0"); + if gpu.gfx_target != target || gpu.is_apu != Some(gfx_is_apu_family(target)) { + wrong.push(format!(" [1002:{id}] is {target}, scanned as {gpu:?}")); + } + } + assert!( + wrong.is_empty(), + "lspci rows the device id identifies but the scan did not:\n{}", + wrong.join("\n") + ); +} + +// --------------------------------------------------------------------------- +// The folds that follow the PCI scan +// --------------------------------------------------------------------------- + +/// The APU verdict survives a `rocminfo` fold whether or not ROCm/rocm-cli#393 +/// has been fixed. +/// +/// #393: `rocminfo` prints indented ISA `Name:` sub-entries after each agent, +/// the parser's running `cur_name` ends up holding +/// `amdgcn-amd-amdhsa--gfx11-generic`, and the agent is dropped. That made the +/// `is_apu` overwrite unreachable on a real host, and fixing #393 is what makes +/// it reachable, so both parser shapes are driven here. +#[test] +fn the_apu_verdict_holds_on_both_sides_of_the_rocminfo_isa_name_defect() { + for (with_isa, parser) in [ + (true, "with the agent-dropping parser of #393"), + (false, "with a parser that reads the agent"), + ] { + let gpu = pci_scanned("Raphael", "164e", "0000:14:00.0"); + assert_eq!(gpu.is_apu, Some(true), "the PCI scan alone knows Raphael"); + let mut e = Examination { + gpus: vec![gpu], + ..Examination::default() + }; + apply_rocminfo_gpu_agents( + &mut e, + &rocminfo_output_shaped(&[("gfx1036", "AMD Radeon Graphics")], with_isa), + ); + summarise_gpu_categories(&mut e); + assert_eq!(e.gpus[0].gfx_target, "gfx1036", "{parser}: {:?}", e.gpus); + assert!( + e.has_apu && !e.has_discrete_amd, + "{parser}: a Raphael-only host is one APU and no discrete GPU: {:?}", + e.gpus + ); + } +} + +/// Reading `rocminfo` only ever improves the report. +/// +/// - A Strix Halo whose `lspci` row identifies nothing: the agent supplies both +/// the target and the APU verdict that follows from it. +/// - A Rembrandt laptop whose agent names a target nothing here classifies: the +/// target is worth recording, but it is not evidence of a discrete GPU, so +/// the PCI scan's verdict stands. +/// - A discrete card the PCI scan could not identify: a known discrete target +/// still revises the default, which keeps the fold from being a no-op. +#[test] +fn folding_in_rocminfo_only_ever_adds_knowledge() { + let host = |lspci_name: &str, device_id: &str, agent_target: &str| { + let mut e = Examination { + gpus: vec![pci_scanned(lspci_name, device_id, "0000:66:00.0")], + ..Examination::default() + }; + apply_rocminfo_gpu_agents( + &mut e, + &rocminfo_output(&[(agent_target, "AMD Radeon Graphics")]), + ); + summarise_gpu_categories(&mut e); + e + }; + + let halo = host("Device", "1586", "gfx1151"); + assert_eq!(halo.gpus[0].gfx_target, "gfx1151", "{:?}", halo.gpus); + assert!( + halo.has_apu && !halo.has_discrete_amd, + "a gfx1151 agent establishes the APU verdict the PCI scan could not: {:?}", + halo.gpus + ); + + let unknown = host("Rembrandt [Radeon 680M]", "1681", "gfx1170"); + assert_eq!(unknown.gpus[0].gfx_target, "gfx1170", "{:?}", unknown.gpus); + assert!( + unknown.has_apu && !unknown.has_discrete_amd, + "an unclassified target must leave the PCI scan's APU verdict standing: {:?}", + unknown.gpus + ); + + // Navi 10 is in neither table, so the scan reports it with the default + // `Some(false)`; a gfx1010 agent confirms it. + let navi10 = host( + "Navi 10 [Radeon RX 5600 OEM/5600 XT / 5700/5700 XT]", + "731f", + "gfx1010", + ); + assert!( + navi10.has_discrete_amd && !navi10.has_apu, + "{:?}", + navi10.gpus + ); +} + +/// The same rule where there is no PCI row at all: an ordinary ROCm container, +/// which ships `rocminfo` but not `pciutils`. +/// +/// Every agent lands in the branch that *creates* a GPU entry, so there is no +/// earlier verdict to preserve -- but "not an APU" and "cannot say" still +/// differ. Defaulting the latter to `Some(false)` reports a discrete AMD GPU on +/// a laptop that has none. +#[test] +fn an_agent_with_no_pci_row_is_classified_from_what_evidence_there_is() { + let container = |agent_target: &str, marketing: &str| { + let mut e = Examination::default(); + apply_rocminfo_gpu_agents(&mut e, &rocminfo_output(&[(agent_target, marketing)])); + summarise_gpu_categories(&mut e); + e + }; + + // The target is enough on its own. + let known = container("gfx1036", "AMD Radeon Graphics"); + assert_eq!(known.gpus[0].is_apu, Some(true), "{:?}", known.gpus); + + // An unclassified target, but the agent names the part: the name answers. + let named = container("gfx1170", "AMD Radeon(TM) 780M Graphics"); + assert_eq!(named.gpus[0].is_apu, Some(true), "{:?}", named.gpus); + assert!(!named.has_discrete_amd, "{:?}", named.gpus); + + // Neither answers. Saying "discrete" here would be inventing a fact. + let silent = container("gfx1170", ""); + assert_eq!(silent.gpus[0].is_apu, None, "{:?}", silent.gpus); + assert!( + silent.has_amd_gpu && !silent.has_apu && !silent.has_discrete_amd, + "{:?}", + silent.gpus + ); + + // A discrete card still reports as one. + let discrete = container("gfx1100", "AMD Radeon RX 7900 XTX"); + assert_eq!(discrete.gpus[0].is_apu, Some(false), "{:?}", discrete.gpus); +} + +/// `hipInfo` revises the display probe's verdict when it reports a target +/// whose packaging is known, and only then. +/// +/// The first half is a host that exists: a Windows Mendocino laptop reports +/// "AMD Radeon(TM) Graphics" with a device id the table does not map, so the +/// display probe gives it no target and the default `Some(false)`, and only +/// `hipInfo`'s `gcnArchName` can say what it is. +/// +/// The second half pins the fold's contract on a state the display probe +/// cannot produce today -- a GPU already called an APU, with no target -- since +/// the probe only reaches `Some(true)` by resolving one. Through the real +/// pipeline the "leave an unknown target's verdict alone" rule therefore +/// changes no output yet; it is asserted so the first new source of a +/// target-less verdict does not inherit the downgrade silently. +#[test] +fn a_hipinfo_target_revises_a_verdict_only_when_its_packaging_is_known() { + let mut e = Examination::default(); + parse_gpus_windows_output(&windows_row("AMD Radeon(TM) Graphics", "1506"), &mut e); + assert_eq!(e.gpus[0].gfx_target, "", "the premise: {:?}", e.gpus); + assert_eq!(e.gpus[0].is_apu, Some(false), "the premise: {:?}", e.gpus); + fill_unidentified_gpus_from_hipinfo(&mut e.gpus, " gcnArchName: gfx1037\n"); + assert_eq!(e.gpus[0].gfx_target, "gfx1037", "{:?}", e.gpus); + assert_eq!( + e.gpus[0].is_apu, + Some(true), + "a known integrated target must revise the default: {:?}", + e.gpus + ); + + let mut gpus = vec![Gpu { + name: "AMD Radeon(TM) Graphics".to_owned(), + is_amd: true, + is_apu: Some(true), + ..Gpu::default() + }]; + fill_unidentified_gpus_from_hipinfo(&mut gpus, " gcnArchName: gfx1170\n"); + assert_eq!(gpus[0].gfx_target, "gfx1170", "{gpus:?}"); + assert_eq!( + gpus[0].is_apu, + Some(true), + "an unclassified target is not evidence of a discrete GPU: {gpus:?}" + ); +} + +/// The never-downgrade rule run through the whole post-PCI sequence against a +/// planted KFD topology, rather than through `apply_rocminfo_gpu_agents` alone. +/// +/// A Ryzen 6800H laptop: `lspci` names the iGPU "Rembrandt [Radeon 680M]", the +/// kernel exposes one KFD node for it at `0000:04:00.0`, and `rocminfo` reports +/// a target nothing here classifies (as a newer ROCm would for a newer part). +/// The report must not call the laptop's only GPU discrete. +#[test] +fn a_rembrandt_laptop_keeps_its_apu_verdict_through_every_probe() { + let root = tempfile::tempdir().expect("scratch dir"); + let nodes = root.path().join("nodes"); + std::fs::create_dir_all(nodes.join("0")).expect("plant the CPU node"); + std::fs::write( + nodes.join("0").join("properties"), + "cpu_cores_count 16\ngfx_target_version 0\nlocation_id 0\ndomain 0\n", + ) + .expect("plant the CPU node properties"); + std::fs::create_dir_all(nodes.join("1")).expect("plant the GPU node"); + std::fs::write( + nodes.join("1").join("properties"), + "simd_count 12\ngfx_target_version 100305\nlocation_id 1024\ndomain 0\n", + ) + .expect("plant the GPU node properties"); + + let mut e = Examination::default(); + apply_lspci_gpus( + &mut e, + "0000:04:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Rembrandt [Radeon 680M] [1002:1681] (rev c8)", + ); + probe_gpus_after_lspci( + &mut e, + GpuProbeSources { + kfd_nodes: &nodes, + rocminfo: Some(&rocminfo_output(&[("gfx1170", "AMD Radeon Graphics")])), + sysfs_gfx_target: || None, + }, + ); + summarise_gpu_categories(&mut e); + + assert_eq!(e.gpus.len(), 1, "one GPU: {:?}", e.gpus); + assert!( + e.has_apu && !e.has_discrete_amd, + "a Radeon 680M iGPU must stay an APU: {:?}", + e.gpus + ); +} + +/// What the iGPU+dGPU collision check sees, run through the real diagnosis +/// catalog. +/// +/// A Ryzen 7000 desktop pairing the Raphael iGPU with an RX 7900 XTX is the +/// host `check_9_igpu_dgpu_collision` exists for, and it fires only when +/// `has_apu && has_discrete_amd`. The note must also name each card correctly, +/// since a diagnosis that confuses the two is no better than silence. +#[test] +fn the_igpu_dgpu_collision_check_fires_on_a_raphael_plus_rx7900_host() { + let igpu = "0000:14:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Raphael [1002:164e] (rev c1)"; + let dgpu = "0000:03:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Navi 31 [Radeon RX 7900 XT/7900 XTX/7900 GRE/7900M] [1002:744c]"; + let mut e = Examination { + os_family: "linux".to_owned(), + ..Examination::default() + }; + apply_lspci_gpus(&mut e, &format!("{dgpu}\n{igpu}\n")); + apply_rocminfo_gpu_agents( + &mut e, + &rocminfo_output(&[ + ("gfx1100", "AMD Radeon RX 7900 XTX"), + ("gfx1036", "AMD Radeon Graphics"), + ]), + ); + summarise_gpu_categories(&mut e); + + let report = crate::diagnose::diagnose(&e, "my training run segfaults"); + let Some(collision) = report + .matched + .iter() + .find(|d| d.id == "fix-9-igpu-dgpu" && d.score > 0) + else { + panic!( + "an iGPU+dGPU host must raise fix-9-igpu-dgpu; has_apu={}, \ + has_discrete_amd={}, gpus={:?}", + e.has_apu, e.has_discrete_amd, e.gpus, + ); + }; + let notes = collision + .fix + .as_ref() + .map(|fix| fix.notes.join(" ")) + .unwrap_or_default(); + assert!( + notes.contains("[\"gfx1100\"]") && notes.contains("[\"gfx1036\"]"), + "the note must name gfx1100 as the discrete GPU and gfx1036 as the APU: {notes}" + ); +} + +/// An AMD PCI row nothing can identify is reported as discrete, on purpose, +/// and both sides of that trade are pinned here -- see +/// `classify_amd_marketing_name`. +/// +/// What it buys: a Ryzen 7000 desktop with an RX 5700 XT. Navi 10 is in +/// neither table; reported as "cannot say", it would clear `has_discrete_amd` +/// and the collision check would never fire on the host it is written for. +/// +/// What it costs: a Strix Halo whose `lspci` row is the nameless `Device` of +/// #449 is reported with `has_discrete_amd` set. The collision check still +/// needs `has_apu`, which that host lacks, so no diagnosis fires; the cost is a +/// wrong field in `rocm examine --json` until `rocminfo` names the target. +#[test] +fn an_unidentifiable_pci_row_is_reported_as_discrete_and_this_is_what_that_costs() { + let raphael = "0000:14:00.0 VGA compatible controller [0300]: Advanced Micro Devices, \ + Inc. [AMD/ATI] Raphael [1002:164e] (rev c1)"; + let navi10 = "0000:03:00.0 VGA compatible controller [0300]: Advanced Micro Devices, \ + Inc. [AMD/ATI] Navi 10 [Radeon RX 5600 OEM/5600 XT / 5700/5700 XT] \ + [1002:731f] (rev c1)"; + let halo = "0000:66:00.0 Display controller [0380]: Advanced Micro Devices, \ + Inc. [AMD/ATI] Device [1002:1586] (rev c1)"; + let host = |lines: &[&str]| { + let mut e = Examination { + os_family: "linux".to_owned(), + ..Examination::default() + }; + apply_lspci_gpus(&mut e, &lines.join("\n")); + summarise_gpu_categories(&mut e); + let fires = crate::diagnose::diagnose(&e, "my training run segfaults") + .matched + .iter() + .any(|d| d.id == "fix-9-igpu-dgpu" && d.score > 0); + (e, fires) + }; + + let (hybrid, fires) = host(&[navi10, raphael]); + assert_eq!( + hybrid.gpus[0].gfx_target, "", + "the premise: Navi 10 is unresolved" + ); + assert!( + hybrid.has_apu && hybrid.has_discrete_amd && fires, + "an unresolved discrete card beside a known iGPU must still raise the \ + collision diagnosis: {:?}", + hybrid.gpus + ); + + let (laptop, fires) = host(&[halo]); + assert_eq!( + laptop.gpus[0].gfx_target, "", + "the premise: the row is unresolved" + ); + assert!( + laptop.has_discrete_amd && !laptop.has_apu, + "the known cost: an unresolved iGPU reads as discrete: {:?}", + laptop.gpus + ); + assert!( + !fires, + "and it stops there: no collision diagnosis on one GPU" + ); +} + +// --------------------------------------------------------------------------- +// Generator reach +// --------------------------------------------------------------------------- + +/// Measure how far the generators reach, by drawing from them directly and +/// tallying which regions each draw lands in. +/// +/// A generator that never samples the interesting region passes every property +/// vacuously, so every counter printed here has a floor. The floors sit well +/// under the measured rates so a reseed does +/// not trip them, and well over zero so a generator that stopped reaching a +/// region does. +#[test] +fn generator_reach_is_measured_and_sufficient() { + const DRAWS: u32 = 2048; + let draws = u64::from(DRAWS); + let mut runner = TestRunner::new(Config::with_cases(DRAWS)); + + let mut lspci_parsed = 0u64; + let mut lspci_gpu_class = 0u64; + let mut lspci_apu_device = 0u64; + let mut lspci_classified_apu = 0u64; + let mut lspci_gfx_resolved = 0u64; + for _ in 0..DRAWS { + let (line, device) = lspci_line() + .new_tree(&mut runner) + .expect("lspci strategy") + .current(); + if is_lspci_gpu_line(&line) { + lspci_gpu_class += 1; + } + if !extract_lspci_name(&line).is_empty() { + lspci_parsed += 1; + } + if device.3 { + lspci_apu_device += 1; + } + let mut e = Examination::default(); + apply_lspci_gpus(&mut e, &line); + if let Some(gpu) = e.gpus.first() { + if gpu.is_apu == Some(true) { + lspci_classified_apu += 1; + } + if !gpu.gfx_target.is_empty() { + lspci_gfx_resolved += 1; + } + } + } + + let mut marketing_resolved = 0u64; + let mut marketing_apu_truth = 0u64; + for _ in 0..DRAWS { + let (name, truth) = marketing_name() + .new_tree(&mut runner) + .expect("marketing strategy") + .current(); + if !classify_amd_marketing_name(&name).0.is_empty() { + marketing_resolved += 1; + } + if truth.2 { + marketing_apu_truth += 1; + } + } + + let mut agent_unclassified = 0u64; + let agent_strategy = proptest::sample::select(apu_lspci_devices()) + .prop_flat_map(|device| agent_target_for(device.2)); + for _ in 0..DRAWS { + let target = agent_strategy + .new_tree(&mut runner) + .expect("agent strategy") + .current(); + if gfx_apu_verdict(target).is_none() { + agent_unclassified += 1; + } + } + + #[allow(clippy::cast_precision_loss)] + let pct = |n: u64| (n as f64) * 100.0 / f64::from(DRAWS); + println!( + "generator reach over {DRAWS} draws each:\n\ + lspci lines : name parsed {:.1}%, GPU-class {:.1}%, APU silicon {:.1}%, \ + scanned as APU {:.1}%, gfx resolved {:.1}%\n\ + marketing names : resolved {:.1}%, APU silicon {:.1}%\n\ + rocminfo agents : unclassified target {:.1}%", + pct(lspci_parsed), + pct(lspci_gpu_class), + pct(lspci_apu_device), + pct(lspci_classified_apu), + pct(lspci_gfx_resolved), + pct(marketing_resolved), + pct(marketing_apu_truth), + pct(agent_unclassified), + ); + let floors = [ + ("lspci: name parsed", lspci_parsed, draws / 2), + // A third, not a half: one class in five is the PCI bridge the probe + // must not enumerate, and the case and tab mutations break the + // case-sensitive class match on purpose so that branch is sampled too. + ("lspci: enumerated GPU class", lspci_gpu_class, draws / 3), + ("lspci: APU silicon", lspci_apu_device, draws / 4), + ("lspci: scanned as an APU", lspci_classified_apu, draws / 6), + ("lspci: gfx target resolved", lspci_gfx_resolved, draws / 4), + ("marketing: name resolved", marketing_resolved, draws / 2), + ("marketing: APU silicon", marketing_apu_truth, draws / 3), + ( + "rocminfo: unclassified agent target", + agent_unclassified, + draws / 8, + ), + ]; + for (region, reached, floor) in floors { + assert!( + reached > floor, + "{region}: {reached} of {DRAWS} draws, under the floor of {floor}" + ); + } +} diff --git a/crates/rocm-core/src/host_gpu.rs b/crates/rocm-core/src/host_gpu.rs index 37f6d632d..4cae3de57 100644 --- a/crates/rocm-core/src/host_gpu.rs +++ b/crates/rocm-core/src/host_gpu.rs @@ -608,7 +608,7 @@ const fn detect_linux_primary_gpu_name() -> Option { None } -fn parse_windows_display_gfx_target(text: &str) -> Option { +pub(crate) fn parse_windows_display_gfx_target(text: &str) -> Option { let mut name_fallback = None; let mut unsupported_match = None; for line in text.lines().map(str::trim).filter(|line| !line.is_empty()) { @@ -625,8 +625,17 @@ fn parse_windows_display_gfx_target(text: &str) -> Option { } return Some(target.to_owned()); } - if name_fallback.is_none() { - name_fallback = gfx_target_from_amd_marketing_name(name).map(str::to_owned); + if name_fallback.is_none() + && let Some(target) = gfx_target_from_amd_marketing_name(name) + { + // The same rule for a name: the marketing table names gfx90c by its + // codenames, and an iGPU recognised that way must not hide a + // discrete card further down the list either. + if gfx_target_has_no_rocm_build(target) { + unsupported_match = unsupported_match.or(Some(target)); + } else { + name_fallback = Some(target.to_owned()); + } } } name_fallback.or_else(|| unsupported_match.map(str::to_owned)) @@ -683,7 +692,7 @@ fn amd_pci_device_id_from_pnp_id(pnp_id: &str) -> Option { } } -fn gfx_target_from_amd_pci_device_id(device_id: &str) -> Option<&'static str> { +pub(crate) fn gfx_target_from_amd_pci_device_id(device_id: &str) -> Option<&'static str> { match device_id.to_ascii_lowercase().as_str() { // Navi 21 / 22 / 23 / 24: Radeon RX 6000 desktop and mobile ASICs. "73a0" | "73a1" | "73a2" | "73a3" | "73a5" | "73a8" | "73a9" | "73ab" | "73ac" | "73ad" @@ -720,7 +729,13 @@ fn gfx_target_from_amd_pci_device_id(device_id: &str) -> Option<&'static str> { } } -fn gfx_target_from_amd_marketing_name(name: &str) -> Option<&'static str> { +/// The gfx target an AMD marketing name or `lspci` codename names, or `None` +/// when no entry in [`AMD_MARKETING_GFX_TARGETS`] matches. +/// +/// This is the crate's one name-to-target table: the install-side display +/// probes and `examine`'s GPU probes both answer from it, so a name resolves to +/// the same target whichever command reads it. +pub(crate) fn gfx_target_from_amd_marketing_name(name: &str) -> Option<&'static str> { let lower = name.to_ascii_lowercase(); let normalized = normalize_marketing_name_for_match(&lower); for entry in AMD_MARKETING_GFX_TARGETS { @@ -732,12 +747,12 @@ fn gfx_target_from_amd_marketing_name(name: &str) -> Option<&'static str> { } #[derive(Debug, Clone, Copy)] -struct AmdMarketingGfxTarget { - pattern: &'static str, - gfx_target: &'static str, +pub(crate) struct AmdMarketingGfxTarget { + pub(crate) pattern: &'static str, + pub(crate) gfx_target: &'static str, } -const AMD_MARKETING_GFX_TARGETS: &[AmdMarketingGfxTarget] = &[ +pub(crate) const AMD_MARKETING_GFX_TARGETS: &[AmdMarketingGfxTarget] = &[ // RDNA4 discrete. AmdMarketingGfxTarget { pattern: "ai pro r9700", @@ -886,6 +901,17 @@ const AMD_MARKETING_GFX_TARGETS: &[AmdMarketingGfxTarget] = &[ pattern: "8040s", gfx_target: "gfx1151", }, + // The codename and the product line, as `lspci` and `rocminfo` print them. + // There is no "8045S": the three Strix Halo SKU names are 8060S, 8050S and + // 8040S (the Ryzen AI Max PRO 380's), so this table never carried one. + AmdMarketingGfxTarget { + pattern: "strix halo", + gfx_target: "gfx1151", + }, + AmdMarketingGfxTarget { + pattern: "ryzen ai max", + gfx_target: "gfx1151", + }, // RDNA3.5 APUs. AmdMarketingGfxTarget { pattern: "890m", @@ -895,6 +921,10 @@ const AMD_MARKETING_GFX_TARGETS: &[AmdMarketingGfxTarget] = &[ pattern: "880m", gfx_target: "gfx1150", }, + AmdMarketingGfxTarget { + pattern: "strix point", + gfx_target: "gfx1150", + }, AmdMarketingGfxTarget { pattern: "860m", gfx_target: "gfx1152", @@ -903,6 +933,13 @@ const AMD_MARKETING_GFX_TARGETS: &[AmdMarketingGfxTarget] = &[ pattern: "840m", gfx_target: "gfx1152", }, + // Krackan Point is gfx1152, as its device id (`1114`) and the 860M/840M it + // sells as both say. gfx1150 and gfx1152 are separate TheRock families, so + // calling it gfx1150 selects the wrong runtime build. + AmdMarketingGfxTarget { + pattern: "krackan", + gfx_target: "gfx1152", + }, AmdMarketingGfxTarget { pattern: "820m", gfx_target: "gfx1153", @@ -920,6 +957,32 @@ const AMD_MARKETING_GFX_TARGETS: &[AmdMarketingGfxTarget] = &[ pattern: "740m", gfx_target: "gfx1103", }, + // `lspci` prints the Phoenix codename with a die number (`Phoenix1`, + // `Phoenix3`), which is a different token from `phoenix`. + AmdMarketingGfxTarget { + pattern: "phoenix", + gfx_target: "gfx1103", + }, + AmdMarketingGfxTarget { + pattern: "phoenix1", + gfx_target: "gfx1103", + }, + AmdMarketingGfxTarget { + pattern: "phoenix2", + gfx_target: "gfx1103", + }, + AmdMarketingGfxTarget { + pattern: "phoenix3", + gfx_target: "gfx1103", + }, + AmdMarketingGfxTarget { + pattern: "phoenix4", + gfx_target: "gfx1103", + }, + AmdMarketingGfxTarget { + pattern: "hawk point", + gfx_target: "gfx1103", + }, // RDNA2 APUs. AmdMarketingGfxTarget { pattern: "680m", @@ -929,10 +992,26 @@ const AMD_MARKETING_GFX_TARGETS: &[AmdMarketingGfxTarget] = &[ pattern: "660m", gfx_target: "gfx1035", }, + AmdMarketingGfxTarget { + pattern: "rembrandt", + gfx_target: "gfx1035", + }, + // Mendocino (Ryzen 7020 series) sells as a Radeon 610M too, which the row + // below answers as gfx1036; only the codename tells the two apart, so it is + // listed ahead of that row for `lspci`'s sake. A Windows Mendocino, named + // only "Radeon(TM) 610M", still reads as gfx1036 by name. + AmdMarketingGfxTarget { + pattern: "mendocino", + gfx_target: "gfx1037", + }, AmdMarketingGfxTarget { pattern: "610m", gfx_target: "gfx1036", }, + AmdMarketingGfxTarget { + pattern: "raphael", + gfx_target: "gfx1036", + }, AmdMarketingGfxTarget { pattern: "steam deck", gfx_target: "gfx1033", @@ -941,6 +1020,37 @@ const AMD_MARKETING_GFX_TARGETS: &[AmdMarketingGfxTarget] = &[ pattern: "van gogh", gfx_target: "gfx1033", }, + // `lspci` spells it as one word, and the Steam Deck's own display name is + // the part number rather than any Radeon model. + AmdMarketingGfxTarget { + pattern: "vangogh", + gfx_target: "gfx1033", + }, + AmdMarketingGfxTarget { + pattern: "custom gpu 0405", + gfx_target: "gfx1033", + }, + // Vega iGPUs in Ryzen 4000/5000/7030 parts. They sell as a bare "AMD + // Radeon Graphics", so the codename `lspci` prints is the only name that + // identifies them. gfx90c has no ROCm build (`gfx_target_has_no_rocm_build`), + // which is why the install-side probes rank a name answering it below every + // other recognised adapter. + AmdMarketingGfxTarget { + pattern: "renoir", + gfx_target: "gfx90c", + }, + AmdMarketingGfxTarget { + pattern: "cezanne", + gfx_target: "gfx90c", + }, + AmdMarketingGfxTarget { + pattern: "lucienne", + gfx_target: "gfx90c", + }, + AmdMarketingGfxTarget { + pattern: "barcelo", + gfx_target: "gfx90c", + }, ]; fn normalize_marketing_name_for_match(value: &str) -> String { @@ -1715,9 +1825,10 @@ impl WindowsExamineInventory { .iter() .find(|display| pci_target(display).is_some_and(|t| !gfx_target_has_no_rocm_build(t))) .or_else(|| { - self.displays - .iter() - .find(|display| gfx_target_from_amd_marketing_name(&display.name).is_some()) + self.displays.iter().find(|display| { + gfx_target_from_amd_marketing_name(&display.name) + .is_some_and(|t| !gfx_target_has_no_rocm_build(t)) + }) }) // A recognized-but-unsupported iGPU (gfx90c) ranks below every // other recognized adapter, so it cannot hide a discrete card. @@ -3607,6 +3718,50 @@ mod tests { ); } + /// A gfx90c iGPU recognised by its *name* must not hide a discrete card + /// either. + /// + /// The name table now answers gfx90c for the Vega iGPUs' codenames, which + /// is what `examine` needs; on the install side that answer has to rank + /// where the PCI-id answer for the same iGPU already does, below every + /// other recognised adapter, or the first adapter's name would decide the + /// install family for the whole host. + #[test] + fn windows_display_parser_does_not_let_a_named_vega_igpu_hide_a_discrete_card() { + let text = "AMD Radeon(TM) Graphics (Renoir)\t\n\ + AMD Radeon RX 6800M\tPCI\\VEN_1002&DEV_9999"; + assert_eq!( + parse_windows_display_gfx_target(text), + Some("gfx1031".to_owned()) + ); + let text = "AMD Radeon(TM) Graphics (Renoir)\t"; + assert_eq!( + parse_windows_display_gfx_target(text), + Some("gfx90c".to_owned()), + "alone, the iGPU is still reported as gfx90c" + ); + + let inventory = WindowsExamineInventory { + displays: vec![ + WindowsDisplayAdapter { + name: "AMD Radeon(TM) Graphics (Renoir)".to_owned(), + driver_version: None, + pnp_device_id: None, + }, + WindowsDisplayAdapter { + name: "AMD Radeon RX 6800M".to_owned(), + driver_version: None, + pnp_device_id: None, + }, + ], + ..WindowsExamineInventory::default() + }; + assert_eq!( + inventory.amd_display_name().as_deref(), + Some("AMD Radeon RX 6800M") + ); + } + #[test] fn known_therock_families_all_round_trip() { for family in known_therock_families() { diff --git a/xtask/src/ascii_output_contract.rs b/xtask/src/ascii_output_contract.rs index f078ba574..1e202a88c 100644 --- a/xtask/src/ascii_output_contract.rs +++ b/xtask/src/ascii_output_contract.rs @@ -205,11 +205,6 @@ mod tests { "\\u{feff}", STRIPPED_FROM_INPUT, ), - ( - "crates/rocm-core/src/examine.rs", - "(\\u{2122})", - STRIPPED_FROM_INPUT, - ), ("crates/rocm-dash-core/src/state.rs", "⋯", JOB_GLYPH), ("crates/rocm-dash-core/src/state.rs", "✓", JOB_GLYPH), ("crates/rocm-dash-core/src/state.rs", "✗", JOB_GLYPH), From 9ef55749070b8e98ceb47b98b33e88100f480595 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Mon, 26 Oct 2026 10:01:04 +0000 Subject: [PATCH 2/2] test(e2e): cover how examine tells integrated GPUs from discrete ones (examine-29) AGENTS.md section 3: what `rocm examine --json` reports for each GPU is user-observable, so it gets a scenario, not only unit tests. The host is simulated, so it runs on every Linux lane, GPU or not: a stand-in `lspci` lists five AMD GPUs as `lspci -nn -D` prints them, an empty `rocminfo` stops a GPU runner's own agents being paired onto those rows, and the simulated host root is empty. Each row reaches its answer by a different route: an RX 7900 XTX by marketing name, a Raphael iGPU by device id alone (the nameless `Device` row), Krackan Point by both, Renoir and Mendocino by codename. Run against main's classifier, every one of the five rows is reported wrong. The rocminfo/hipInfo never-downgrade rule is not reachable from here, since a real rocminfo's agents are dropped by #393; docs/testing.md says where it is covered instead. docs/manual-testing.md gains the check for a real integrated GPU. Signed-off-by: Roman Inflianskas --- docs/manual-testing.md | 23 +++ docs/testing.md | 31 ++++ tests/e2e-cucumber/features/examine.feature | 23 +++ tests/e2e-cucumber/tests/e2e/examine_steps.rs | 139 ++++++++++++++++++ 4 files changed, 216 insertions(+) diff --git a/docs/manual-testing.md b/docs/manual-testing.md index f29df5486..7ce4a8982 100644 --- a/docs/manual-testing.md +++ b/docs/manual-testing.md @@ -254,6 +254,29 @@ fallback paired an architecture line with the wrong GPU. On that machine `rocm diagnose` must **not** report `info-20-gpu-no-rocm-support` at all — the discrete card does have ROCm support, so the whole-machine claim is false. +### 2b. Integrated or discrete + +On a machine with an AMD integrated GPU, check what `rocm examine --json` +reports for it under `gpus`: + +```bash +rocm examine --framework skip --json +``` + +Expected result: + +- Each AMD GPU carries a `gfx_target`, and an integrated one carries + `is_apu: true`. On a Ryzen 7000 or later desktop the iGPU is `gfx1036`; on a + Ryzen 6000 laptop `gfx1035`; on a Krackan Point laptop (Radeon 860M or 840M) + `gfx1152`, never `gfx1150`; on a Ryzen 4000 or 5000 machine `gfx90c`. +- A discrete card beside it carries `is_apu: false`, and `has_apu` and + `has_discrete_amd` are both `true`. An iGPU reported with `is_apu: false`, or + an iGPU-only machine with `has_discrete_amd: true`, is a regression unless + the GPU's `gfx_target` is empty (an unidentified row is reported as discrete + on purpose). +- On a machine whose only GPU is the iGPU, `rocm serve` does not warn about low + VRAM. + ## 3. Lemonade GPU Verification Lemonade is the default local assistant/server engine. Serve a small assistant diff --git a/docs/testing.md b/docs/testing.md index 30b1486e4..a2a171292 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1845,6 +1845,37 @@ for them, including for the fallback; their parsing is covered by the unit tests above, and the fallback itself by `hardware_root`'s own unit tests, since both now read through the same helper with an identity resolver. +## Integrated Or Discrete GPU + +`rocm examine` reports each AMD GPU's `gfx_target` and `is_apu`, and two things +a user sees follow from `is_apu`: `rocm serve` withholds its low-VRAM warning +on a single-APU host, whose "VRAM" is a BIOS carve-out of system RAM, and +`rocm diagnose` raises `fix-9-igpu-dgpu` only when a host has both an +integrated and a discrete AMD GPU. + +`examine-29` in [examine.feature](../tests/e2e-cucumber/features/examine.feature) +covers the PCI scan end to end on every Linux lane, GPU or not. It puts a +stand-in `lspci` and an empty `rocminfo` on `PATH` and points the simulated host +root at an empty tree, so the five planted rows are the only GPUs: one each +classified by marketing name, by device id alone (the `Device` row `lspci` +prints when `pci.ids` has no name), by both, and by codename. + +```bash +RUSTC_WRAPPER= cargo xtask e2e -- -n 'examine-29 ' +``` + +What `rocminfo` and `hipInfo` add is covered one level down, in +[examine_gpu_proptests.rs](../crates/rocm-core/src/examine_gpu_proptests.rs): +a target nothing classifies must leave an earlier verdict standing +(`gfx_apu_verdict` answers `None` for it). That rule cannot be reached end to +end today, because a real `rocminfo` prints ISA `Name:` lines that make the +parser drop every agent (#393). The property harness asserts floors on how much +of each corpus its generators reach, and prints the measured rates: + +```bash +cargo test -p rocm-core --lib -- examine::gpu_proptests --nocapture +``` + ## Windows Runtime Preflight On Windows, `rocm diagnose` compares the Visual C++ runtime in `System32` diff --git a/tests/e2e-cucumber/features/examine.feature b/tests/e2e-cucumber/features/examine.feature index 00c62c3f3..e5a26787e 100644 --- a/tests/e2e-cucumber/features/examine.feature +++ b/tests/e2e-cucumber/features/examine.feature @@ -428,3 +428,26 @@ Feature: GPU detection and system inspection Given a GPU machine whose release file cannot be read When the user inspects that machine in machine-readable form Then the inspection's verdict is degraded, and its only failure says the release file could not be read + + # Whether a GPU is integrated decides two things a user sees: `rocm serve` + # withholds its low-VRAM warning on an APU (whose "VRAM" is a BIOS carve-out + # of system RAM), and `rocm diagnose` raises `fix-9-igpu-dgpu` only on a host + # with both an integrated and a discrete AMD GPU. Every AMD GPU on this bus + # used to be misreported: the PCI scan ignored the `[1002:xxxx]` device id, + # so a GPU `pci.ids` cannot name ("Device") got no architecture; `examine` + # kept its own name table, which called Krackan Point gfx1150 (it is gfx1152) + # and knew the Renoir and Mendocino codenames only as "some APU"; and gfx1037 + # was not classified as integrated at all. + # + # The host is simulated, so this runs on every Linux lane, GPU or not. A + # stand-in `lspci` on PATH lists the five GPUs as `lspci -nn -D` prints them, + # a stand-in `rocminfo` lists no agents, and the simulated host root has no + # kernel topology, so the PCI scan alone names every card. (What `rocminfo` + # adds is covered one level down: on a real host its agents are dropped by + # ROCm/rocm-cli#393, so the folds it feeds cannot be reached from here.) + @id:examine-tells-integrated-gpus-from-discrete-ones @requires-os:linux + Scenario: examine-29 - The inspection tells integrated GPUs from discrete ones by the part they are + Given a machine whose PCI bus lists a Radeon RX 7900 XTX and four integrated AMD GPUs + When the user inspects that machine in machine-readable form + Then the inspection names each GPU's architecture and reports only the RX 7900 XTX as discrete + And the inspection reports the machine as having both an integrated and a discrete AMD GPU diff --git a/tests/e2e-cucumber/tests/e2e/examine_steps.rs b/tests/e2e-cucumber/tests/e2e/examine_steps.rs index 95f0344aa..62cc8bd64 100644 --- a/tests/e2e-cucumber/tests/e2e/examine_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/examine_steps.rs @@ -1457,6 +1457,145 @@ async fn gpu_machine_with_unreadable_release_file(world: &mut E2eWorld) { plant_gpu_fixtures(world); } +/// The five AMD GPUs `examine-29` puts on the PCI bus, as `lspci -nn -D` +/// prints them: `(PCI address, gfx target, integrated, lspci line)`. +/// +/// Each row reaches its answer by a different door, so each one fails alone if +/// that door regresses: the RX 7900 XTX by its marketing name (its device id is +/// not in the table), the Raphael iGPU by its device id alone (`pci.ids` has no +/// name for it here), Krackan Point by both (and the name used to say gfx1150), +/// Renoir and Mendocino by their codenames. +const PCI_BUS_GPUS: &[(&str, &str, bool, &str)] = &[ + ( + "0000:03:00.0", + "gfx1100", + false, + "0000:03:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Navi 31 [Radeon RX 7900 XT/7900 XTX/7900 GRE/7900M] [1002:744c] (rev c8)", + ), + ( + "0000:14:00.0", + "gfx1036", + true, + "0000:14:00.0 Display controller [0380]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Device [1002:164e] (rev c1)", + ), + ( + "0000:c5:00.0", + "gfx1152", + true, + "0000:c5:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Krackan Point [Radeon 860M] [1002:1114] (rev c1)", + ), + ( + "0000:04:00.0", + "gfx90c", + true, + "0000:04:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Renoir [1002:1636] (rev c6)", + ), + ( + "0000:05:00.0", + "gfx1037", + true, + "0000:05:00.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. \ + [AMD/ATI] Mendocino [1002:1506] (rev c1)", + ), +]; + +#[given("a machine whose PCI bus lists a Radeon RX 7900 XTX and four integrated AMD GPUs")] +async fn machine_with_five_amd_gpus_on_the_bus(world: &mut E2eWorld) { + let root = world + .isolated_root + .as_ref() + .expect("no isolated root") + .path() + .to_owned(); + // An empty simulated host root: no kernel topology, no sysfs GPU, so + // nothing but the PCI scan names a card, and no real card on the runner + // can join the list. + let host_root = root.join("host-root"); + std::fs::create_dir_all(&host_root).expect("failed to create the simulated host root"); + let bin = root.join("fake-bin"); + std::fs::create_dir_all(&bin).expect("failed to create the fake PATH dir"); + let listing = PCI_BUS_GPUS + .iter() + .map(|(_, _, _, line)| *line) + .collect::>() + .join("\n"); + rocm_core::fake_executable::write_fake_executable( + &bin.join("lspci"), + format!("#!/bin/sh\ncat <<'LSPCI'\n{listing}\nLSPCI\n"), + ) + .expect("failed to write the fake lspci"); + // A `rocminfo` that answers and lists no agents, ahead of any real one, so + // a GPU runner's own agents are not paired onto these rows. + rocm_core::fake_executable::write_fake_executable(&bin.join("rocminfo"), "#!/bin/sh\nexit 0\n") + .expect("failed to write the fake rocminfo"); + let path = std::env::join_paths(std::iter::once(bin).chain(std::env::split_paths( + &std::env::var_os("PATH").unwrap_or_default(), + ))) + .expect("PATH entries cannot hold a separator"); + world.command_env.push(("PATH", path)); + world + .command_env + .push(("ROCM_CLI_TEST_HOST_ROOT", host_root.into_os_string())); +} + +#[then("the inspection names each GPU's architecture and reports only the RX 7900 XTX as discrete")] +async fn assert_each_gpu_classified(world: &mut E2eWorld) { + assert_eq!(world.cli_rc, Some(0), "`rocm examine --json` failed"); + let json = parsed_json(world); + let gpus = json + .get("gpus") + .and_then(serde_json::Value::as_array) + .unwrap_or_else(|| panic!("`examine --json` has no gpus array:\n{json:#}")); + let amd: Vec<&serde_json::Value> = gpus + .iter() + .filter(|gpu| gpu.get("is_amd").and_then(serde_json::Value::as_bool) == Some(true)) + .collect(); + assert_eq!( + amd.len(), + PCI_BUS_GPUS.len(), + "one AMD GPU per planted PCI row and no other:\n{json:#}" + ); + let mut wrong = Vec::new(); + for (address, target, integrated, _) in PCI_BUS_GPUS { + let Some(gpu) = amd + .iter() + .find(|gpu| gpu.get("pci_id").and_then(serde_json::Value::as_str) == Some(address)) + else { + wrong.push(format!("{address}: not reported")); + continue; + }; + let reported_target = gpu.get("gfx_target").and_then(serde_json::Value::as_str); + let reported_integrated = gpu.get("is_apu").and_then(serde_json::Value::as_bool); + if reported_target != Some(target) || reported_integrated != Some(*integrated) { + wrong.push(format!( + "{address}: expected {target}, integrated={integrated}; got \ + gfx_target={reported_target:?}, is_apu={reported_integrated:?}" + )); + } + } + assert!( + wrong.is_empty(), + "GPUs reported with the wrong architecture or packaging:\n{}\n{json:#}", + wrong.join("\n") + ); +} + +#[then("the inspection reports the machine as having both an integrated and a discrete AMD GPU")] +async fn assert_both_gpu_categories(world: &mut E2eWorld) { + let json = parsed_json(world); + for field in ["has_amd_gpu", "has_apu", "has_discrete_amd"] { + assert_eq!( + json.get(field).and_then(serde_json::Value::as_bool), + Some(true), + "{field}:\n{json:#}" + ); + } +} + #[when("the user inspects that machine in machine-readable form")] async fn user_inspects_planted_machine(world: &mut E2eWorld) { let (stdout, stderr, rc) =