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

[no-reply@sashiko.virtuozzo.com]

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?

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

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261007-b4-prep-khorenko-vstor-148656-cpuid-v6-0-a8eb906e69b8@virtuozzo.com?part=1

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