On 2/23/24 13:30, Moger, Babu wrote:
> Hi Ilpo,
> 
> On 2/23/24 04:53, Ilpo Järvinen wrote:
>> On Thu, 22 Feb 2024, Babu Moger wrote:
>>
>>> Add support to read UMC (Unified Memory Controller) perf events to compare
>>> the numbers with QoS monitor for AMD.
>>>
>>> Signed-off-by: Babu Moger <babu.mo...@amd.com>
>>> ---
>>>  .../testing/selftests/resctrl/resctrl_tests.c |  6 +-
>>>  tools/testing/selftests/resctrl/resctrl_val.c | 62 +++++++++++++++++--
>>>  2 files changed, 58 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c 
>>> b/tools/testing/selftests/resctrl/resctrl_tests.c
>>> index 2bbe3045a018..231233b8d354 100644
>>> --- a/tools/testing/selftests/resctrl/resctrl_tests.c
>>> +++ b/tools/testing/selftests/resctrl/resctrl_tests.c
>>> @@ -104,8 +104,7 @@ static void run_mbm_test(const char * const 
>>> *benchmark_cmd, int cpu_no)
>>>     }
>>>  
>>>     if (!validate_resctrl_feature_request("L3_MON", "mbm_total_bytes") ||
>>> -       !validate_resctrl_feature_request("L3_MON", "mbm_local_bytes") ||
>>> -       (get_vendor() != ARCH_INTEL)) {
>>> +       !validate_resctrl_feature_request("L3_MON", "mbm_local_bytes")) {
>>>             ksft_test_result_skip("Hardware does not support MBM or MBM is 
>>> disabled\n");
>>>             goto cleanup;
>>>     }
>>> @@ -131,8 +130,7 @@ static void run_mba_test(const char * const 
>>> *benchmark_cmd, int cpu_no)
>>>     }
>>>  
>>>     if (!validate_resctrl_feature_request("MB", NULL) ||
>>> -       !validate_resctrl_feature_request("L3_MON", "mbm_local_bytes") ||
>>> -       (get_vendor() != ARCH_INTEL)) {
>>> +       !validate_resctrl_feature_request("L3_MON", "mbm_local_bytes")) {
>>>             ksft_test_result_skip("Hardware does not support MBA or MBA is 
>>> disabled\n");
>>>             goto cleanup;
>>>     }
>>
>> These need to be rebased as this code moved into per test files with the 
>> generic test framework. The .vendor_specific field is the new way to make 
>> tests limited to particular vendors.
> 
> Hmm. I picked the latest code from tip/master. Where is the latest code
> now? Is it yet to be submitted?
> 
> I can wait for your code to merge first.

Oh. ok. Here it is
https://lore.kernel.org/lkml/cover.1708072203.git.maciej.wieczor-ret...@intel.com/

I will rebase it on top of this next revision.

Thanks
Babu

> 
>>
>>> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c 
>>> b/tools/testing/selftests/resctrl/resctrl_val.c
>>> index 792116d3565f..c5a4607aa9d9 100644
>>> --- a/tools/testing/selftests/resctrl/resctrl_val.c
>>> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
>>> @@ -11,6 +11,7 @@
>>>  #include "resctrl.h"
>>>  
>>>  #define UNCORE_IMC         "uncore_imc"
>>> +#define AMD_UMC                    "amd_umc"
>>>  #define READ_FILE_NAME             "events/cas_count_read"
>>>  #define WRITE_FILE_NAME            "events/cas_count_write"
>>>  #define DYN_PMU_PATH               "/sys/bus/event_source/devices"
>>> @@ -146,6 +147,47 @@ static int open_perf_event(int i, int cpu_no, int j)
>>>     return 0;
>>>  }
>>>  
>>> +/* Get type and config (read and write) of an UMC counter */
>>> +static int read_from_umc_dir(char *umc_dir, int count)
>>> +{
>>> +   char umc_counter_type[1024];
>>
>> PATH_MAX
> 
> ok.
>>
>>> +   FILE *fp;
>>> +
>>> +   /* Get type of iMC counter */
>>> +   sprintf(umc_counter_type, "%s%s", umc_dir, "type");
>>> +   fp = fopen(umc_counter_type, "r");
>>> +   if (!fp) {
>>> +           perror("Failed to open imc counter type file");
>>
>> Please don't add new perror() anymore, I just managed to clean up those
>> in favor of ksft_perror().
> 
> Ok. Will look into it.
>>
>>> +
>>
>> Drop this newline (to slowly move towards more concise error handling 
>> blocks w/o all those extra newlines).
> 
> ok.
> 
>>
>>> +           return -1;
>>
>>
>>> +   }
>>> +   if (fscanf(fp, "%u", &imc_counters_config[count][READ].type) <= 0) {
>>> +           perror("Could not get imc type");
>>
>> Ditto.
> 
> ok
> 
>>
>>> +           fclose(fp);
>>> +           return -1;
>>> +   }
>>> +
>>> +   fclose(fp);
>>> +
>>> +   imc_counters_config[count][WRITE].type =
>>> +           imc_counters_config[count][READ].type;
>>> +
>>> +   /*
>>> +    * Setup the event and umasks for UMC events
>>> +    * Number of CAS commands sent for reads:
>>> +    * EventCode = 0x0a, umask = 0x1
>>> +    * Number of CAS commands sent for writes:
>>> +    * EventCode = 0x0a, umask = 0x2
>>> +    */
>>> +   imc_counters_config[count][READ].event = 0xa;
>>> +   imc_counters_config[count][READ].umask = 0x1;
>>> +
>>> +   imc_counters_config[count][WRITE].event = 0xa;
>>> +   imc_counters_config[count][WRITE].umask = 0x2;
>>> +
>>> +   return 0;
>>> +}
>>> +
>>>  /* Get type and config (read and write) of an iMC counter */
>>>  static int read_from_imc_dir(char *imc_dir, int count)
>>>  {
>>
> 

-- 
Thanks
Babu Moger

Reply via email to