linux-kernel - Re: [PATCH] selftests: resctrl: ignore builds for unsupported architectures

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <db16db55-5f68-484f-ba9f-3312b41bf426@intel.com>
Date: Mon, 12 Aug 2024 13:36:16 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: 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>, Shuah Khan
	<skhan@...uxfoundation.org>, 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



On 8/9/24 1:45 AM, 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?
> 
> 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)?
> 

It is not obvious to me that resctrl needs those "trivial program" compile
tests. For testing the target architecture ARCH seems appropriate. I do not
think it is guaranteed that ARCH will always be set though so the Makefile
may need an additional snippet to set ARCH to "uname -m" if it is not provided by
environment, similar to what is done in other Makefiles.

Reinette