On 8/12/24 18:05, Reinette Chatre wrote:
Hi Shuah,
On 8/12/24 3:49 PM, Shuah Khan wrote:
On 8/9/24 02:45, Ilpo Järvinen wrote:
Adding Maciej.
On Fri, 9 Aug 2024, Muhammad Usama Anjum wrote:
On 8/9/24 12:23 PM, Ilpo Järvinen wrote:
On Fri, 9 Aug 2024, Muhammad Usama Anjum wrote:
This test doesn't have support for other architectures. Altough resctrl
is supported on x86 and ARM, but arch_supports_noncont_cat() shows that
only x86 for AMD and Intel are supported by the test.
One does not follow from the other. arch_supports_noncont_cat() is only
small part of the tests so saying "This test" based on a small subset of
all tests is bogus. Also, I don't see any reason why ARCH_ARM could not be
added and arch_supports_noncont_cat() adapted accordingly.
I'm not familiar with resctrl and the architectural part of it. Feel
free to fix it and ignore this patch.
If more things are missing than just adjusting
arch_supports_noncont_cat(), the test should be turned off until proper
support is added to the test.
We get build
errors when built for ARM and ARM64.
As this seems the real reason, please quote any errors when you use them
as justification so it can be reviewed if the reasoning is sound or not.
CC resctrl_tests
In file included from resctrl.h:24,
from cat_test.c:11:
In function 'arch_supports_noncont_cat',
inlined from 'noncont_cat_run_test' at cat_test.c:323:6:
../kselftest.h:74:9: error: impossible constraint in 'asm'
74 | __asm__ __volatile__ ("cpuid\n\t"
\
| ^~~~~~~
cat_test.c:301:17: note: in expansion of macro '__cpuid_count'
301 | __cpuid_count(0x10, 1, eax, ebx, ecx, edx);
| ^~~~~~~~~~~~~
../kselftest.h:74:9: error: impossible constraint in 'asm'
74 | __asm__ __volatile__ ("cpuid\n\t"
\
| ^~~~~~~
cat_test.c:303:17: note: in expansion of macro '__cpuid_count'
303 | __cpuid_count(0x10, 2, eax, ebx, ecx, edx);
| ^~~~~~~~~~~~~
Okay, so it's specific to lack of CPUID. This seems a kselftest common
level problem to me, since __cpuid_count() is provided in kselftest.h.
Shuah (or others), what is the intended mechanism for selftests to know if
it can be used or not since as is, it's always defined?
_cpuid_count() gets defined in ksefltest.h if it can't find it.
As the comment says both gcc and cland probide __cpuid_count()
gcc cpuid.h provides __cpuid_count() since v4.4.
Clang/LLVM cpuid.h provides __cpuid_count() since v3.4.0.
I see some Makefiles use compile testing a trivial program to decide whether
they build some x86_64 tests or not. Is that what should be done here too,
test if __cpuid_count() compiles or not (and then build some #ifdeffery
based on the result of that compile testing)?
These build errors need to be fixed instead of restricting the build> In some cases when the test can't be supported on an architecture then it is okay
to suppress build. This is not a general solution to suppress build warnings
While there is an effort to support Arm in resctrl [1], this is not currently
the case and the resctrl selftests as a consequence only support x86 with
built-in assumptions that a test runs on either AMD or Intel. After the kernel gains support
for Arm more changes will be needed for the resctrl tests to support another architecture
so I do think the most appropriate change to address this build failure is to restrict
resctrl tests to x86.
Sounds good to me. This would be good case for suppressing test build.
I would recommend against adding suppress build code when it can be fixed.
I expect after resctrl fs obtains support for Arm the resctrl selftests can be
updated to support it with more fine grained architectural checks than a global
enable/disable needed at this time.
Let's investigate this problem to fix it properly. I don't see any arm and arm64
maintainers and developers on this thread. It would be good to investigate to
see if this can be fixed.
thanks,
-- Shuah
