fix(docs): address review findings — accuracy, consistency, completeness

- architecture.md: fix kernel param math to match actual computed values, use cardN placeholder in sysfs paths, clarify system_ram_kb is OS-visible - benchmarking.md: normalize flags to -ngl 99 / -mmp 0 (matching code), add llama-rocm7-nightlies backend - CLAUDE.md: clarify HSA_OVERRIDE_GFX_VERSION is set in containers not scripts, fix lib sourcing description, specify which scripts need root - detect.sh: document detect_cpu_cores returns threads not cores - troubleshooting.md: add link to references.md - README.md: remove unsupported Fedora 42 claim, describe configs/ content Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-25 21:44:16 +01:00
parent 5b81437637
commit da2c4c6b8a
6 changed files with 41 additions and 29 deletions
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -4,14 +4,14 @@ Optimization toolkit for AMD Strix Halo (Ryzen AI MAX+ 395, Radeon 8060S gfx1151

 ## Architecture

-`bin/` dispatchers → `scripts/` implementations → `lib/` shared libraries. All scripts source libs in order: `common.sh` → `detect.sh` → `format.sh`. Runtime data goes to `data/` (gitignored). Full details in [docs/architecture.md](docs/architecture.md).
+`bin/` dispatchers → `scripts/` implementations → `lib/` shared libraries. Scripts source libs as needed: always `common.sh` first, then `detect.sh` if hardware detection is needed, then `format.sh` if formatted output is needed. Some scripts (e.g., `rollback.sh`) only need `common.sh`. Runtime data goes to `data/` (gitignored). Full details in [docs/architecture.md](docs/architecture.md).

 ## Safety Rules

 - **`scripts/optimize/kernel-params.sh`** modifies `/etc/default/grub` — requires root, backs up to `data/backups/` first. Always maintain the Python-with-env-vars pattern for GRUB editing (no shell variable interpolation into Python code).
 - **`scripts/optimize/tuned-profile.sh`** and **`rollback.sh`** require root and save previous state for rollback.
 - **`data/backups/`** contains GRUB backups and tuned profile snapshots — never delete these.
- Optimization scripts that require root check `$EUID` at the top and exit immediately if not root.
+- Optimization scripts that modify system state (`kernel-params.sh`, `tuned-profile.sh`, `rollback.sh`) check `$EUID` at the top and exit immediately if not root. Guidance-only scripts (`vram-gtt.sh`, `verify.sh`) do not require root.
 - All Python blocks receive data via environment variables (`os.environ`), never via shell interpolation into Python source. This prevents injection. **Do not revert to `'''$var'''` or `"$var"` patterns inside Python heredocs.**

 ## Key Technical Details
@@ -21,8 +21,8 @@ Optimization toolkit for AMD Strix Halo (Ryzen AI MAX+ 395, Radeon 8060S gfx1151
 - **Kernel param detection**: `detect_kernel_param()` uses word-boundary-anchored regex to avoid `iommu` matching `amd_iommu`.
 - **Benchmark invocation**: `toolbox run -c NAME -- [env ROCBLAS_USE_HIPBLASLT=1] /path/to/llama-bench -ngl 99 -mmp 0 -fa 1 -r N`. ENV_ARGS passed as a proper bash array (not string splitting).
 - **llama-bench output**: Pipe-delimited table. Python parser at fixed column indices (parts[8]=test, parts[9]=t/s). Format changes upstream would break parsing.
- **ROCm for gfx1151**: `ROCBLAS_USE_HIPBLASLT=1`, `HSA_OVERRIDE_GFX_VERSION=11.5.1`.
- **Fedora GRUB**: Prefers `grubby` (BLS) over `grub2-mkconfig`. Both paths are handled.
+- **ROCm for gfx1151**: Scripts set `ROCBLAS_USE_HIPBLASLT=1` in benchmark ENV_ARGS. `HSA_OVERRIDE_GFX_VERSION=11.5.1` is set inside the toolbox containers (not by our scripts) — needed for ollama and native ROCm builds.
+- **Fedora GRUB**: Prefers `grubby` (BLS), falls back to `grub2-mkconfig`, then `grub-mkconfig`. All three paths are handled.

 ## Conventions

--- a/README.md
+++ b/README.md
@@ -84,7 +84,7 @@ See [docs/optimization.md](docs/optimization.md) for the full walkthrough with e
 bin/            Entry points (audit, monitor, benchmark, optimize)
 lib/            Shared bash libraries (common, detect, format)
 scripts/        Implementation organized by function
-configs/        Reference configuration templates
+configs/        Reference configuration (grub-cmdline.conf with recommended kernel params)
 data/           Runtime output: audits, benchmarks, logs, backups (gitignored)
 docs/           Technical documentation
 ```
@@ -93,7 +93,7 @@ See [docs/architecture.md](docs/architecture.md) for the full architecture, data

 ## Requirements

- **OS**: Fedora 43 (tested), Fedora 42+ should work
+- **OS**: Fedora 43 (tested). Requires kernel >= 6.18.4
 - **Hardware**: AMD Strix Halo (Ryzen AI MAX / MAX+) with RDNA 3.5 iGPU
 - **Tools**: `bc`, `python3`, `tmux`, `podman`, `toolbox`
 - **Optional**: `amdgpu_top` (installed via `make monitor-install`), `huggingface-cli` (for model downloads)
--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -25,7 +25,7 @@ data/                   Runtime output (gitignored)
 docs/                   Documentation
 ```

-Every script sources libs in order: `common.sh` → `detect.sh` → `format.sh`. `format.sh` depends on color variables defined in `common.sh`.
+Scripts source libs as needed: always `common.sh` first, then `detect.sh` for hardware detection, then `format.sh` for formatted output. Not all scripts need all three — `rollback.sh` only needs `common.sh`, for example. `format.sh` depends on color variables defined in `common.sh`.

 ## Data Flow

@@ -54,33 +54,40 @@ AMD Strix Halo shares physical RAM between CPU and GPU. Two allocation mechanism

 **Optimal for LLM workloads**: Minimize VRAM (0.5 GiB), maximize GTT (~60 GiB on 64 GB system). The GPU borrows memory when needed and releases it when idle.

-### Kernel Parameter Math (64 GB system)
+### Kernel Parameter Math
+
+The formula (using integer GiB arithmetic):

 ```
-Total physical RAM:     64 GiB
-OS reserve:              4 GiB
-Available for GTT:      60 GiB = 61440 MiB
-
-amdgpu.gttsize = 60 * 1024         = 61440   (MiB)
-ttm.pages_limit = 60 * 1024 * 256  = 15728640 (4K pages)
-iommu = pt                                     (passthrough, lower latency)
+total_physical_gib = (visible_ram + dedicated_vram) / 1024 / 1024   (integer division)
+gtt_gib            = total_physical_gib - 4                          (4 GiB OS reserve)
+amdgpu.gttsize     = gtt_gib * 1024                                  (MiB)
+ttm.pages_limit    = amdgpu.gttsize * 256                             (4K pages)
+iommu              = pt                                                (passthrough)
 ```

-The toolkit computes these dynamically via `recommended_gttsize_mib()` and `recommended_pages_limit()` in `lib/detect.sh`, based on detected total physical RAM (visible + VRAM).
+Actual values depend on how the OS reports memory. On a 64 GB HP ZBook with 32 GB dedicated VRAM:
+
+```
+visible_ram ≈ 31.1 GiB, vram = 32 GiB → total ≈ 63 GiB (integer: 59 GiB GTT)
+amdgpu.gttsize = 60416, ttm.pages_limit = 15466496
+```
+
+Run `make audit` to see the exact computed values for your system. The toolkit computes these dynamically via `recommended_gttsize_mib()` and `recommended_pages_limit()` in `lib/detect.sh`.

 ### Sysfs Paths

+Card number is auto-detected by `find_gpu_card()` in `lib/detect.sh` (matches AMD vendor ID `0x1002`, falls back to first card with `mem_info_vram_total`). Below uses `cardN` as placeholder:
+
 | Path | Content |
 |------|---------|
-| `/sys/class/drm/card1/device/mem_info_vram_total` | Dedicated VRAM in bytes |
-| `/sys/class/drm/card1/device/mem_info_vram_used` | VRAM currently in use |
-| `/sys/class/drm/card1/device/mem_info_gtt_total` | GTT allocation in bytes |
-| `/sys/class/drm/card1/device/mem_info_gtt_used` | GTT currently in use |
-| `/sys/class/drm/card1/device/gpu_busy_percent` | GPU utilization 0-100 |
-| `/sys/class/drm/card1/device/hwmon/hwmon*/temp1_input` | Temperature in millidegrees C |
-| `/sys/class/drm/card1/device/hwmon/hwmon*/power1_average` | Power in microwatts |
-
-Card number is auto-detected by `find_gpu_card()` (matches AMD vendor ID `0x1002`).
+| `/sys/class/drm/cardN/device/mem_info_vram_total` | Dedicated VRAM in bytes |
+| `/sys/class/drm/cardN/device/mem_info_vram_used` | VRAM currently in use |
+| `/sys/class/drm/cardN/device/mem_info_gtt_total` | GTT allocation in bytes |
+| `/sys/class/drm/cardN/device/mem_info_gtt_used` | GTT currently in use |
+| `/sys/class/drm/cardN/device/gpu_busy_percent` | GPU utilization 0-100 |
+| `/sys/class/drm/cardN/device/hwmon/hwmon*/temp1_input` | Temperature in millidegrees C |
+| `/sys/class/drm/cardN/device/hwmon/hwmon*/power1_average` | Power in microwatts |

 ## JSON Output Schemas

@@ -89,7 +96,7 @@ Card number is auto-detected by `find_gpu_card()` (matches AMD vendor ID `0x1002
 ```json
 {
  "timestamp": "20260325-120000",
-  "hardware": { "cpu_model": "...", "cpu_cores": 16, "cpu_threads": 32, "gpu_name": "...", "gpu_device_id": "1586", "system_ram_kb": 32609248 },
+  "hardware": { "cpu_model": "...", "cpu_cores": 16, "cpu_threads": 32, "gpu_name": "...", "gpu_device_id": "1586", "system_ram_kb": 32609248 },  // OS-visible RAM (excludes dedicated VRAM)
  "memory": { "vram_total_bytes": 0, "vram_used_bytes": 0, "gtt_total_bytes": 0, "gtt_used_bytes": 0, "recommended_gttsize_mib": 0, "recommended_pages_limit": 0 },
  "kernel": { "version": "...", "cmdline": "...", "param_iommu": "", "param_gttsize": "", "param_pages_limit": "" },
  "firmware": "...", "tuned_profile": "...", "rocm_version": "...",
--- a/docs/benchmarking.md
+++ b/docs/benchmarking.md
@@ -32,7 +32,7 @@ Results are in **tokens/second (t/s)**. Higher is better.
 - `-ub SIZE` — micro-batch size (512 for Vulkan, 2048 for ROCm)
 - `-r 3` — 3 repetitions (long-context tests are slow)

-The `-fa 1 --no-mmap -ngl 999` flags are **mandatory** on Strix Halo to avoid crashes.
+The `-fa 1 -mmp 0 -ngl 99` flags are **mandatory** on Strix Halo to avoid crashes (`-fa 1` = flash attention, `-mmp 0` = no memory mapping, `-ngl 99` = all layers on GPU).

 ## Available Backends

@@ -42,6 +42,7 @@ The `-fa 1 --no-mmap -ngl 999` flags are **mandatory** on Strix Halo to avoid cr
 | `llama-vulkan-amdvlk` | AMDVLK | Vulkan | Fastest when it works, 2GB buffer limit |
 | `llama-rocm-6.4.4` | ROCm 6.4.4 | HIP | Proven stable |
 | `llama-rocm-7.2` | ROCm 7.2 | HIP | Latest, compiler fixes applied |
+| `llama-rocm7-nightlies` | ROCm 7 nightly | HIP | Experimental/development builds |

 Containers are from [kyuz0/amd-strix-halo-toolboxes](https://github.com/kyuz0/amd-strix-halo-toolboxes). Set up with `make benchmark-setup`.

--- a/docs/troubleshooting.md
+++ b/docs/troubleshooting.md
@@ -94,3 +94,7 @@ sudo make rollback
 ```

 This restores the GRUB backup and previous tuned profile. BIOS changes must be reverted manually (F10 at boot). See [docs/optimization.md](optimization.md) for the full rollback procedure.
+
+## Further Resources
+
+For external tool documentation, upstream bug trackers, and community resources, see [docs/references.md](references.md).
--- a/lib/detect.sh
+++ b/lib/detect.sh
@@ -24,8 +24,8 @@ GPU_SYSFS="$(find_gpu_card)"

 # --- CPU ---
 detect_cpu_model()   { grep -m1 'model name' /proc/cpuinfo | cut -d: -f2 | xargs; }
-detect_cpu_cores()   { grep -c '^processor' /proc/cpuinfo; }
-detect_cpu_physical() { grep 'cpu cores' /proc/cpuinfo | head -1 | cut -d: -f2 | xargs; }
+detect_cpu_cores()   { grep -c '^processor' /proc/cpuinfo; }  # logical processors (threads)
+detect_cpu_physical() { grep 'cpu cores' /proc/cpuinfo | head -1 | cut -d: -f2 | xargs; }  # physical cores

 # --- GPU ---
 detect_gpu_name() {