Hi Babu, On 4/25/2024 1:17 PM, 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> > --- > tools/testing/selftests/resctrl/resctrl_val.c | 67 ++++++++++++++++--- > 1 file changed, 59 insertions(+), 8 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c > index e3b09128ec3d..d90d3196d7b5 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[PATH_MAX]; > + FILE *fp; > + > + /* Get type of iMC counter */ iMC counter? > + sprintf(umc_counter_type, "%s%s", umc_dir, "type"); > + fp = fopen(umc_counter_type, "r"); > + if (!fp) { > + ksft_perror("Failed to open imc counter type file"); Why go through effort of changing to generic names and then follow by using Intel naming in AMD specific code? > + return -1; > + } > + > + if (fscanf(fp, "%u", &imc_counters_config[count][READ].type) <= 0) { > + ksft_perror("Could not get imc type"); Same here. > + fclose(fp); > + return -1; > + } > + > + fclose(fp); > + > + imc_counters_config[count][WRITE].type = > + imc_counters_config[count][READ].type; > + Up to here seems to be a copy&paste of read_from_imc_dir(). Could you instead split read_from_imc_dir() so that AMD and Intel can share the code to determine type? Reinette