Skip to content

Added tests for dmabuf validations#296

Open
vnarapar wants to merge 1 commit intoqualcomm-linux:mainfrom
vnarapar:dmabuf
Open

Added tests for dmabuf validations#296
vnarapar wants to merge 1 commit intoqualcomm-linux:mainfrom
vnarapar:dmabuf

Conversation

@vnarapar
Copy link
Contributor

Added the tests to check DMA configs, Device Tree Validation and upstream kselftests

Added the tests to check DMA configs, Device Tree Validation and
upstream kselftests

Signed-off-by: Vamsee Narapareddi <vnarapar@qti.qualcomm.com>
@vnarapar vnarapar requested a review from smuppand February 15, 2026 19:17
@@ -0,0 +1,21 @@
metadata:
format: Lava-Test Test Definition 1.0
name: dmabuf_config
Copy link
Contributor

Choose a reason for hiding this comment

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

Set YAML name: dmabuf (or rename test folder + TESTNAME + run-test invocation). Don’t mix.


### 1. Kernel Configuration Validation
**Mandatory**:
- `CONFIG_DMA_SHARED_BUFFER CONFIG_DMABUF_HEAPS CONFIG_DMABUF_HEAPS_SYSTEM CONFIG_TEE_DMABUF_HEAPS CONFIG_HAS_DMA` - Core DMA-BUF support
Copy link
Contributor

Choose a reason for hiding this comment

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

In run.sh they are WARN-only (good), but README shows FAIL lines for optional configs. That’s confusing. Update README expected output to show WARN/INFO for optional configs only.


log_info "=== Kernel Configuration Validation ==="

CORE_CONFIGS="CONFIG_DMA_SHARED_BUFFER CONFIG_DMABUF_HEAPS CONFIG_DMABUF_HEAPS_SYSTEM CONFIG_TEE_DMABUF_HEAPS CONFIG_HAS_DMA"
Copy link
Contributor

Choose a reason for hiding this comment

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

Core config list is likely too strict / not portable

You require:
CONFIG_TEE_DMABUF_HEAPS as mandatory
CONFIG_DMABUF_HEAPS_SYSTEM mandatory (fine if your baseline expects it)
CONFIG_HAS_DMA mandatory (almost always yes, but not the best signal)

On some upstream configs, TEE_DMABUF_HEAPS might not exist or may be module/disabled but DMA-BUF is still valid.
Recommendation Split “mandatory” into tiers:

Tier-1 mandatory (should fail if absent):
CONFIG_DMA_SHARED_BUFFER
CONFIG_DMABUF_HEAPS (if you’re standardizing on heaps)
optionally CONFIG_DMABUF_HEAPS_SYSTEM only if you require “system heap must exist”.

Tier-2 recommended (warn):
CONFIG_TEE_DMABUF_HEAPS
CONFIG_DMA_HEAP (note: this symbol name may differ across kernels; see below)
CONFIG_DMA_CMA (warn)

Also: there’s a confusion in README between CONFIG_DMA_HEAP and CONFIG_DMA_HEAP vs heaps framework. On upstream kernels, the feature is often CONFIG_DMABUF_HEAPS and the device nodes are /dev/dma_heap/*. Some trees have CONFIG_DMA_HEAP too, but not universal.

done
fi

for heap_path in /proc/device-tree/soc*/dma* /proc/device-tree/soc*/qcom,ion* /proc/device-tree/ion* ; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use dt related functions which are already available in functestlib.sh


CORE_CONFIGS="CONFIG_DMA_SHARED_BUFFER CONFIG_DMABUF_HEAPS CONFIG_DMABUF_HEAPS_SYSTEM CONFIG_TEE_DMABUF_HEAPS CONFIG_HAS_DMA"

if ! check_kernel_config "$CORE_CONFIGS"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing dependency checks / permissions handling

This script assumes:
/proc/config.gz exists (functestlib check_kernel_config uses it)debugfs not needed here

listing /dev/dma_heap doesn’t need root (usually ok)
Use check_dependencies style (you already have it in functestlib).

echo "$TESTNAME FAIL" > "$res_file"
fi
log_info "-------------------Completed $TESTNAME Testcase----------------------------"
exit 0 No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Kernel-functional extensions for dmabuf

Right now it’s “presence validation”. To make it functional without requiring special userspace:

Extension 1: Quick allocate/free sanity (if you have an allocator interface)
If /dev/dma_heap/system exists, you can do a minimal allocation test using upstream tool if present, otherwise skip:

Some environments have dma-heap test tools; most don’t.
So best is: detect helper binary (or your own tiny C helper in repo later).

For now:
Add a hook: “if dmabuf_heap_kselftest exists → suggest running that for functional coverage” (but don’t fail).

Extension 2: dmabuf sysfs/debug info (optional)
If present:
/sys/kernel/debug/dma_buf/bufinfo or similar (varies)
/proc/slabinfo for dmabuf-related caches (too noisy though)

Treat as INFO/WARN only.

B) Review: dmabuf_heap_kselftest (runs upstream selftest)

What’s good

Good CLI flag parser (--binary-path), POSIX-friendly.

Captures output to a file and prints it (good for LAVA artifacts and debugging).

Handles TAP totals line if present; otherwise counts ok/not ok/skip.

Issues / fixes needed

  1. README and usage mismatch

README says:

./run.sh /custom/path/to/dmabuf-heap

But the script does not accept positional path (it ignores positional args, only supports -b/--binary-path). It even prints:

log_info " ./run.sh /path/to/dmabuf-heap"

That instruction is wrong.

Fix Either:

support positional path (simpler), OR

fix README + log message to match --binary-path.

I’d recommend supporting positional path because it’s convenient and backwards compatible:

if $1 doesn’t start with -, treat it as binary path.

  1. OUTPUTFILE.res name is confusing and clashes with “.res”

You create:

output_file="./OUTPUTFILE.res"

That’s confusing in this repo because .res is the final verdict file. Also you delete it at end, so it’s not even an artifact.

Fix Use mktemp or ./dmabuf_heap_kselftest.out (or ./logs/... if you have standard log dirs). Better:

output_file="$(mktemp "$TESTNAME.XXXXXX.log")" with trap cleanup

or keep it and don’t delete so LAVA can archive it.

Given your earlier preferences: usually you keep logs.

  1. TAP parsing is fragile (spaces + grep -o portability)

This part:

pass_count=$(echo "$totals_line" | grep -o " pass:[0-9]*" | cut -d':' -f2)

grep -o isn’t guaranteed on very minimal busybox (often present, but not always).

The pattern includes leading space; ok, but still fragile.

Fix Parse with awk reliably:

extract numbers after pass: fail: etc.

Example robust parse:

pass_count=$(echo "$totals_line" | awk -F'pass:| ' '{print $2}' | awk -F' ' '{print $1}')

or one awk script.

  1. Counting ok lines double-counts SKIPs

Fallback:

pass_count=$(grep -c "^ok " ...)
skip_count=$(grep -c "# SKIP" ...)

But TAP marks skip as ok N # SKIP ..., so pass_count includes skipped tests. That leads to inflated pass count.

Fix When using fallback mode:

pass = ok lines that are NOT SKIP

skip = ok lines that are SKIP

Example:

skip_count=$(grep -c "^ok .*# SKIP" "$output_file")

pass_count=$(grep -c "^ok " "$output_file") - skip_count

  1. Exit code handling: inconsistent with “PASS if totals say fail=0”

You set:

pass=false if fail_count>0 OR error_count>0 OR exit_code!=0 OR pass_count==0.

That’s fine, but note:

Some selftests exit non-zero even when TAP shows pass (rare, but possible), or vice versa.

Best practice: trust TAP totals first; if totals show fail=0 and error=0, treat as pass even if exit code non-zero but log warn.

Fix Priority:

  1. If totals exist: use totals as source of truth; log exit code as info/warn.

  2. If no totals: use exit code + ok/not ok.

  1. Missing dependency checks

You use grep, awk, cut, mktemp not used but would be good, basename, etc.

Fix Add:

deps_list="grep awk cut basename date" (whatever your functestlib expects)

If missing -> SKIP, not FAIL.

@@ -0,0 +1,193 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is already the “functional” one. A few enhancements to increase coverage and reduce skips:

Extension 1: VGEM handling (optional)

Selftest often skips if VGEM not present. You can:

check config CONFIG_DRM_VGEM and module presence

if module exists, try modprobe vgem (only if root) and rerun or continue.

But careful: modprobe may not exist in Yocto minimal images → gate it.

Extension 2: Heap coverage report

If /dev/dma_heap/* has multiple heaps, print them before running selftest and mention which heaps selftest iterated.

Extension 3: Preserve output log artifact

Instead of deleting the output log, keep it and name it:

./dmabuf_heap_kselftest.log This is extremely useful in LAVA triage.

@@ -0,0 +1,21 @@
metadata:
Copy link
Contributor

Choose a reason for hiding this comment

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

Both yaml files use:

  • cd $PWD/suites/Kernel/Baseport/...

This assumes LAVA’s $PWD is Runner/. Often it’s the repo root.

Recommended alignment (your standard style):

set REPO_PATH="$PWD"

cd "$REPO_PATH/Runner/suites/Kernel/Baseport/"

run ./run.sh

call Runner/utils/send-to-lava.sh .res

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants