| Message ID | 3c2034e3391634b35192819b69eabd7db8cffa8f.1717626661.git.babu.moger@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/resctrl: Enable MBM and MBA tests on AMD | expand |
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? Reinette
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.
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 {
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(-)