Hi Reinette,
On 6/14/24 13:39, Reinette Chatre wrote:
Hi Babu,
On 6/5/24 3:45 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@amd.com
v3: Made read_from_mc_dir function generic to both AMD and Intel. Rest are mostly related to rebase.
v2: Replace perror with ksft_perror.
tools/testing/selftests/resctrl/resctrl_val.c | 80 ++++++++++++------- 1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 23c0e0a1d845..ffacafb535cd 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" @@ -128,7 +129,7 @@ static int open_perf_event(int i, int cpu_no, int j) } /* Get type and config (read and write) of an MC counter */ -static int read_from_mc_dir(char *mc_dir, int count) +static int read_from_mc_dir(char *mc_dir, int count, int vendor) { char cas_count_cfg[1024], mc_counter_cfg[1024], mc_counter_type[1024]; FILE *fp; @@ -152,41 +153,56 @@ static int read_from_mc_dir(char *mc_dir, int count) mc_counters_config[count][WRITE].type = mc_counters_config[count][READ].type; - /* Get read config */ - sprintf(mc_counter_cfg, "%s%s", mc_dir, READ_FILE_NAME); - fp = fopen(mc_counter_cfg, "r"); - if (!fp) { - ksft_perror("Failed to open MC config file"); + if (vendor == ARCH_AMD) { + /* + * 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 + */ + mc_counters_config[count][READ].event = 0xa; + mc_counters_config[count][READ].umask = 0x1; - return -1; - } - if (fscanf(fp, "%s", cas_count_cfg) <= 0) { - ksft_perror("Could not get MC cas count read"); + mc_counters_config[count][WRITE].event = 0xa; + mc_counters_config[count][WRITE].umask = 0x2; + } else { + /* Get read config */ + sprintf(mc_counter_cfg, "%s%s", mc_dir, READ_FILE_NAME); + fp = fopen(mc_counter_cfg, "r"); + if (!fp) { + ksft_perror("Failed to open MC config file");
+ return -1; + } + if (fscanf(fp, "%s", cas_count_cfg) <= 0) { + ksft_perror("Could not get MC cas count read"); + fclose(fp);
+ return -1; + } fclose(fp); - return -1; - } - fclose(fp); + get_event_and_umask(cas_count_cfg, count, READ); - get_event_and_umask(cas_count_cfg, count, READ); + /* Get write config */ + sprintf(mc_counter_cfg, "%s%s", mc_dir, WRITE_FILE_NAME); + fp = fopen(mc_counter_cfg, "r"); + if (!fp) { + ksft_perror("Failed to open MC config file"); - /* Get write config */ - sprintf(mc_counter_cfg, "%s%s", mc_dir, WRITE_FILE_NAME); - fp = fopen(mc_counter_cfg, "r"); - if (!fp) { - ksft_perror("Failed to open MC config file"); + return -1; + } + if (fscanf(fp, "%s", cas_count_cfg) <= 0) { + ksft_perror("Could not get MC cas count write"); + fclose(fp); - return -1; - } - if (fscanf(fp, "%s", cas_count_cfg) <= 0) { - ksft_perror("Could not get MC cas count write"); + return -1; + } fclose(fp); - return -1; + get_event_and_umask(cas_count_cfg, count, WRITE); } - fclose(fp);
- get_event_and_umask(cas_count_cfg, count, WRITE); return 0; } @@ -213,6 +229,8 @@ static int num_of_mem_controllers(void) vendor = get_vendor(); if (vendor == ARCH_INTEL) { sysfs_name = UNCORE_IMC; + } else if (vendor == ARCH_AMD) { + sysfs_name = AMD_UMC; } else { ksft_perror("Unsupported vendor!\n"); return -1; @@ -228,6 +246,7 @@ static int num_of_mem_controllers(void) /* * imc counters are named as "uncore_imc_<n>", hence * increment the pointer to point to <n>. + * For AMD, it will be amd_umc_<n>. */ temp = temp + strlen(sysfs_name); @@ -239,7 +258,7 @@ static int num_of_mem_controllers(void) if (temp[0] >= '0' && temp[0] <= '9') { sprintf(mc_dir, "%s/%s/", DYN_PMU_PATH, ep->d_name); - ret = read_from_mc_dir(mc_dir, count); + ret = read_from_mc_dir(mc_dir, count, vendor); if (ret) { closedir(dp); @@ -250,8 +269,9 @@ static int num_of_mem_controllers(void) } closedir(dp); if (count == 0) { - ksft_print_msg("Unable to find MC counters\n");
+ ksft_print_msg("Unable to find iMC/UMC counters\n"); + if (vendor == ARCH_AMD) + ksft_print_msg("Try loading amd_uncore module\n"); return -1; } } else {
Can all the vendor checking be contained in num_of_mem_controllers() instead of scattered through multiple layers? There can be two vendor specific functions to initialize mc_counters_config[][]. Only the type setting code ends up being shared so that can be split into a function that is called by both vendor functions?
Yes, We can do that. Will add it in v4.