Re: [Devel] [PATCH hci-8.1 v6 1/5] selftests/ve: check CPUID override emulation for VE tasks

[Konstantin Khorenko <khorenko@virtuozzo.com>]

On 10/7/26 14:04, no-reply@sashiko.virtuozzo.com wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] selftests/ve: arch gate runs $(CC) -dumpmachine before lib.mk sets CC
> --
> 
> commit de465213924e8ab9123a95613dab45358f011917
> Author: Konstantin Khorenko <khorenko@virtuozzo.com>
> 
> selftests/ve: check CPUID override emulation for VE tasks
> 
> This adds ve_cpuid_override_test, which writes a one-entry
> /proc/vz/cpuid_override table and compares what a task in a fresh VE and
> the host see for CPUID leaf 1. A second test flushes the table after the
> task was armed and expects the hardware value back. The test is added to
> the ve selftests Makefile for x86-64 builds only.
> 
> Link: https://virtuozzo.atlassian.net/browse/VSTOR-148843
> 
>> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
>> index c6efe7c4b4fb..3f494a49270e 100644
>> --- a/tools/testing/selftests/ve/Makefile
>> +++ b/tools/testing/selftests/ve/Makefile
>> @@ -6,4 +6,10 @@ TEST_GEN_PROGS += ve_ns_owner_test
>>  TEST_GEN_PROGS += ve_perms_test
>>  TEST_GEN_PROGS += ve_mount_accounting_test
>>  
>> +# CPUID faulting is x86-64 only. ARCH says "x86" for 32-bit too and lib.mk
>> +# picks the clang target by its emptiness, so decide by the compiler.
>> +ifneq ($(filter x86_64-%,$(shell $(CC) -dumpmachine 2>/dev/null)),)
> 
> [Severity: Low]
> Is $(CC) the right compiler to query at this point? This ifneq is expanded
> while the Makefile is being parsed, before the include of ../lib.mk, and
> lib.mk is where the real compiler is assigned:
> 
> tools/testing/selftests/lib.mk:
>     CC := $(CLANG) $(CLANG_FLAGS) -fintegrated-as    (LLVM builds)
>     CC := $(CROSS_COMPILE)gcc                        (otherwise)
> 
> Until then $(CC) is make's built-in "cc", unless CC was set on the command
> line or in the environment.
> 
> For example, with a cross build from an x86_64 host:
> 
>     make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
> 
> "cc -dumpmachine" prints x86_64-linux-gnu, so the filter matches and
> ve_cpuid_override_test is added to TEST_GEN_PROGS. It is then built with
> the aarch64 compiler, where <cpuid.h> and __cpuid_count are not available,
> so the whole ve selftest directory fails to build.
> 
> The reverse case also looks possible. On an arm64 host cross-building for
> x86_64, the test would be skipped.
> 
> The comment above the check says it decides "by the compiler", and the
> commit message says "The test is built on x86-64 only". Neither seems to
> hold for cross builds.
> 
> Could the check be done after including lib.mk, or be based on ARCH,
> SRCARCH or uname -m instead?

  The observation is right: at that point $(CC) is still make's built-in
  "cc", lib.mk assigns the real compiler only later. The check cannot move
  below the include, though. lib.mk derives its rules from TEST_GEN_PROGS
  while it is being parsed ("all: $(TEST_GEN_PROGS) ..." and the per-program
  rules), so anything added to TEST_GEN_PROGS after the include is simply
  not built.

  The alternatives do not work either:

  - ARCH cannot tell 32-bit from 64-bit x86: the top-level selftests
    Makefile maps both i386 and x86_64 to "x86". Assigning ARCH ourselves
    is out as well, lib.mk uses an empty ARCH to mean "the host target" for
    LLVM=1 builds, and a value it has no CLANG_TARGET_FLAGS_ entry for
    (e.g. "aarch64" from uname -m) makes it error out for the whole ve
    directory. That was the v2 version of this gate, and the review rightly
    rejected it.
  - SRCARCH has the same granularity as ARCH.
  - uname -m knows nothing about cross builds at all, so it is strictly
    worse than asking a compiler.

  What remains is a gap for cross builds only: the gate asks the host
  compiler, so cross-building these selftests from an x86-64 host for
  another architecture would add the test and fail on <cpuid.h>, and the
  reverse would skip it. We do not cross build the vz selftests, the
  feature under test exists on x86-64 only, and I would rather keep a
  three-line gate than grow it to cover a configuration nobody runs. If a
  cross build of tools/testing/selftests/ve ever becomes a thing, the gate
  can be revisited then.

> 
>> +TEST_GEN_PROGS += ve_cpuid_override_test
>> +endif
>> +
>>  include ../lib.mk
> 

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel