Skip to content

build: gate amdgpu plugin on amdgpu dev support#2970

Open
cheese-cakee wants to merge 2 commits intocheckpoint-restore:criu-devfrom
cheese-cakee:fix/issue-1900-libdrm-amdgpu-detect
Open

build: gate amdgpu plugin on amdgpu dev support#2970
cheese-cakee wants to merge 2 commits intocheckpoint-restore:criu-devfrom
cheese-cakee:fix/issue-1900-libdrm-amdgpu-detect

Conversation

@cheese-cakee
Copy link
Copy Markdown
Contributor

Problem

CONFIG_AMDGPU was enabled whenever libdrm was detected, even on systems where libdrm is present without amdgpu development support (libdrm/amdgpu.h / -ldrm_amdgpu).

This causes build failures in plugins/amdgpu/amdgpu_plugin.c.

Change

  • Add FEATURE_TEST_LIBDRM_AMDGPU in scripts/feature-tests.mak to verify both headers and linkability for amdgpu (-ldrm -ldrm_amdgpu).
  • Gate CONFIG_AMDGPU in Makefile.config on that feature test instead of generic pkg-config-check,libdrm.
  • Keep behavior minimal: amdgpu plugin is built only when amdgpu dev support is available.

Notes

This is a surgical build-detection fix with no runtime logic changes.

Fixes: #1900

Check for libdrm amdgpu headers/libraries before enabling
CONFIG_AMDGPU. This avoids build failures on systems where
libdrm is installed without amdgpu support.

Fixes: checkpoint-restore#1900
Signed-off-by: Farzan Aman Khan <[email protected]>
Copilot AI review requested due to automatic review settings March 17, 2026 08:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes build failures when libdrm is installed without amdgpu development support (issue #1900). Previously, CONFIG_AMDGPU was gated on a generic pkg-config-check,libdrm, which succeeded even without amdgpu headers/libraries.

Changes:

  • Added a FEATURE_TEST_LIBDRM_AMDGPU compile test that verifies amdgpu headers and linkability
  • Replaced the pkg-config-check,libdrm gate with a try-cc feature test using -ldrm -ldrm_amdgpu
  • Updated the help message to mention "amdgpu support"

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
scripts/feature-tests.mak New FEATURE_TEST_LIBDRM_AMDGPU compile test checking amdgpu headers and API
Makefile.config Gate CONFIG_AMDGPU on the new feature test instead of generic libdrm pkg-config

You can also share your feedback on Copilot code review. Take the survey.

Include pkg-config libdrm cflags in FEATURE_TEST_LIBDRM_AMDGPU
detection so xf86drm/drm.h include paths are resolved correctly
on distro layouts that require -I/usr/include/libdrm.

Signed-off-by: Farzan Aman Khan <[email protected]>
@rst0git
Copy link
Copy Markdown
Member

rst0git commented Mar 17, 2026

@cheese-cakee To build the AMD GPU plugin, we currently require libdrm-devel / libdrm-dev as a build dependency, and without this package the plugin will be skipped.

CONFIG_AMDGPU was enabled whenever libdrm was detected, even on systems where libdrm is present without amdgpu development support (libdrm/amdgpu.h / -ldrm_amdgpu). This causes build failures in plugins/amdgpu/amdgpu_plugin.c.

Would you be able to provide more information on how to reproduce this error?

@cheese-cakee
Copy link
Copy Markdown
Contributor Author

cheese-cakee commented Mar 17, 2026

Thanks for the question @rst0git . Repro is the one documented in #1900 : libdrm installed but AMDGPU development bits absent (libdrm/amdgpu.h and -ldrm_amdgpu), while generic libdrm detection still enabled CONFIG_AMDGPU; build then fails in plugins/amdgpu/amdgpu_plugin.c. This patch makes detection explicit (header + link), so the plugin is built only when AMDGPU dev support is present. If this case is no longer possible in current CI/package constraints, I'm happy to close as obsolete.

@rst0git
Copy link
Copy Markdown
Member

rst0git commented Mar 17, 2026

Repro is the one documented in #1900

@cheese-cakee This issue was opened in 2022, and the code has changed significantly since then. Were you able to reproduce the error yourself?

@cheese-cakee
Copy link
Copy Markdown
Contributor Author

I reproduced this on current code in a clean Ubuntu 24.04 container.

What I tested:

  • Installed build-essential pkg-config libdrm-dev
  • Simulated the problematic package state (libdrm present, AMDGPU dev bits absent) by removing:
    • /usr/include/libdrm/amdgpu.h
    • /usr/lib/x86_64-linux-gnu/libdrm_amdgpu.so

Observed results:

  • On current criu-dev logic:
    • make -pn | grep CONFIG_AMDGPU -> CONFIG_AMDGPU = y
    • plugin compile fails:
      • plugins/amdgpu/amdgpu_plugin.c:22:10: fatal error: libdrm/amdgpu.h: No such file or directory
  • On this PR branch:
    • make -pn | grep CONFIG_AMDGPU -> no CONFIG_AMDGPU (disabled as expected)

So with libdrm detected but missing AMDGPU dev support, current logic still enables AMDGPU, while this patch gates it correctly.

@cheese-cakee
Copy link
Copy Markdown
Contributor Author

cheese-cakee commented Mar 17, 2026

for clarity:

docker run --rm -it \
  -v "$PWD:/src:ro" ubuntu:24.04 bash -lc '
set -eux
apt-get update -qq
DEBIAN_FRONTEND=noninteractive apt-get install -y -qq build-essential pkg-config libdrm-dev
cp -a /src /work

# Simulate: libdrm present, AMDGPU dev bits missing
mv /usr/include/libdrm/amdgpu.h /usr/include/libdrm/amdgpu.h.bak || true
mv /usr/lib/x86_64-linux-gnu/libdrm_amdgpu.so /usr/lib/x86_64-linux-gnu/libdrm_amdgpu.so.bak || true

# Current criu-dev behavior: generic libdrm check still enables AMDGPU
(cd /work && make -pn 2>/dev/null | grep -m1 CONFIG_AMDGPU || true)

# This then fails in amdgpu plugin compilation
(gcc $(pkg-config --cflags libdrm) -c /work/plugins/amdgpu/amdgpu_plugin.c -o /tmp/amdgpu.o) || true
'

@rst0git
Copy link
Copy Markdown
Member

rst0git commented Mar 17, 2026

# Simulate: libdrm present, AMDGPU dev bits missing
mv /usr/include/libdrm/amdgpu.h /usr/include/libdrm/amdgpu.h.bak || true
mv /usr/lib/x86_64-linux-gnu/libdrm_amdgpu.so /usr/lib/x86_64-linux-gnu/libdrm_amdgpu.so.bak || true

These lines look like intentionally breaking parts of the AMD GPU userspace stack :)

@cheese-cakee
Copy link
Copy Markdown
Contributor Author

cheese-cakee commented Mar 17, 2026

@rst0git I reran this with a package-native setup (no manual deletion of distro files):

  • Built and installed libdrm from source with AMDGPU disabled (-Damdgpu=disabled) into an isolated prefix.
  • Verified in that prefix:
    • pkg-config --exists libdrm -> yes
    • include/libdrm/amdgpu.h -> missing
    • libdrm_amdgpu.so -> missing

Results:

  • Old logic equivalent (pkg-config-check,libdrm) would enable CONFIG_AMDGPU.
  • New feature-test logic fails as expected:
    • fatal error: libdrm/amdgpu.h: No such file or directory
    • therefore CONFIG_AMDGPU stays disabled.

So this reproduces the exact mismatch using a real package build configuration, without mutating installed distro package contents.

@cheese-cakee
Copy link
Copy Markdown
Contributor Author

Adding exact code context that this PR changes:

Old gate (current criu-dev):

ifeq ($(call pkg-config-check,libdrm),y)
export CONFIG_AMDGPU := y
endif

This only checks generic libdrm presence.

New gate (Makefile.config):

LIBDRM_CFLAGS := $(shell $(PKG_CONFIG) --cflags libdrm 2>/dev/null)
ifeq ($(call try-cc,$(FEATURE_TEST_LIBDRM_AMDGPU),$(LIBDRM_CFLAGS) -ldrm -ldrm_amdgpu),true)
export CONFIG_AMDGPU := y
endif

Feature test (scripts/feature-tests.mak):

#include <xf86drm.h>
#include <libdrm/amdgpu.h>
...
return amdgpu_device_initialize(-1, &major, &minor, &dev);

So we now require both headers and linkability to AMDGPU userspace before enabling the plugin.

@avagin avagin requested a review from fdavid-amd March 19, 2026 15:42
@cheese-cakee
Copy link
Copy Markdown
Contributor Author

cheese-cakee commented Mar 24, 2026

Hi @fdavid-amd, following up on the review request from @avagin. The change is a surgical build-detection fix: adds a compile-time check for libdrm/amdgpu.h and ldrm_amdgpu before enabling CONFIG_AMDGPU.

Happy to answer any questions.

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.

Build failure if libdrm installed without amdgpu support

3 participants