[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6dd1b5ce-2ce2-4d61-beff-a100da213528@intel.com>
Date: Mon, 12 Aug 2024 17:05:30 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Shuah Khan <skhan@...uxfoundation.org>, Ilpo Järvinen
<ilpo.jarvinen@...ux.intel.com>, Muhammad Usama Anjum
<Usama.Anjum@...labora.com>
CC: Fenghua Yu <fenghua.yu@...el.com>, Shaopeng Tan
<tan.shaopeng@...fujitsu.com>, <kernel@...labora.com>, LKML
<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
Maciej Wieczór-Retman <maciej.wieczor-retman@...el.com>
Subject: Re: [PATCH] selftests: resctrl: ignore builds for unsupported
architectures
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.
>
> 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.
Reinette
[1] https://lore.kernel.org/lkml/20240802172853.22529-1-james.morse@arm.com/
Powered by blists - more mailing lists