Re: [PATCH v7 10/13] selftests/resctrl: Add vendor detection mechanism

[James Morse <james.morse@arm.com>]

Hi Andre,

(thanks for digging into this!)

On 10/05/2019 18:39, Andre Przywara wrote:
> On Sat,  9 Feb 2019 18:50:39 -0800
> Fenghua Yu <fenghua.yu@intel.com> wrote:
>> From: Babu Moger <babu.moger@amd.com>
>>
>> RESCTRL feature is supported both on Intel and AMD now. Some features
>> are implemented differently. Add vendor detection mechanism. Use the vendor
>> check where there are differences.
> 
> I don't think vendor detection is the right approach. The Linux userland
> interface should be even architecture agnostic, not to speak of different
> vendors.
> 
> But even if we need this for some reason ...

>> diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
>> index 3959b2b0671a..1d9adcfbdb4c 100644
>> --- a/tools/testing/selftests/resctrl/resctrl_tests.c
>> +++ b/tools/testing/selftests/resctrl/resctrl_tests.c
>> @@ -14,6 +14,66 @@
>>  #define BENCHMARK_ARGS		64
>>  #define BENCHMARK_ARG_SIZE	64
>>  
>> +/**
>> + * This variables provides information about the vendor
>> + */
>> +int genuine_intel;
>> +int authentic_amd;
>> +
>> +void lcpuid(const unsigned int leaf, const unsigned int subleaf,
>> +	    struct cpuid_out *out)
> 
> There is a much easier way to detect the vendor, without resorting to
> (unchecked) inline assembly in userland:

> I changed this to scan /proc/cpuinfo for a line starting with vendor_id,
> then use the information there. This should work everywhere.

Everywhere x86. /proc/cpuinfo is unfortunately arch specific, arm64 spells that field 'CPU
implementer'. Short of invoking lscpu, I don't know a portable way of finding this string.

What do we need it for? Surely it indicates something is wrong with the kernel interface
if you need to know which flavour of CPU this is.

The only differences I've spotted are the 'MAX_MBA_BW' on Intel is a percentage, on AMD
its a number between 1 and 2K. If user-space needs to know we could add another file with
the 'max_bandwidth'. (I haven't spotted how the MBA default_ctrl gets exposed).

The other one is the non-contiguous CBM values, where user-space is expected to try it and
see [0]. If user-space needs to know, we could expose some 'sparse_bitmaps=[0 1]', unaware
user-space keeps working.


Thanks,

James

[0] https://lore.kernel.org/patchwork/patch/991236/#1179944