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.moger@xxxxxxx> > --- > .../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. > 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 > + 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(). > + Drop this newline (to slowly move towards more concise error handling blocks w/o all those extra newlines). > + return -1; > + } > + if (fscanf(fp, "%u", &imc_counters_config[count][READ].type) <= 0) { > + perror("Could not get imc type"); Ditto. > + 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) > { -- i.