2024-06-05 22:46:51

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v3 0/4] selftests/resctrl: Enable MBM and MBA tests on AMD


The MBM (Memory Bandwidth Monitoring) and MBA (Memory Bandwidth Allocation)
features are not enabled for AMD systems. The reason was lack of perf
counters to compare the resctrl test results.

Starting with the commit
25e56847821f ("perf/x86/amd/uncore: Add memory controller support"), AMD
now supports the UMC (Unified Memory Controller) perf events. These events
can be used to compare the test results.

This series adds the support to detect the UMC events and enable MBM/MBA
tests for AMD systems.

v3:
Note: Based the series on top of latest kselftests/master
1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 (tag: v6.10-rc1).

Also applied the patches from the series
https://lore.kernel.org/lkml/[email protected]/

Separated the fix patch.
Renamed the imc to just mc to make it generic.
Changed the search string "uncore_imc_" and "amd_umc_"
Changes related rebase to latest kselftest tree.

v2: Changes.
a. Rebased on top of tip/master (Apr 25, 2024)
b. Addressed Ilpo comments except the one about close call.
It seems more clear to keep READ and WRITE separate.
https://lore.kernel.org/lkml/[email protected]/
c. Used ksft_perror call when applicable.
d. Added vendor check for non contiguous CBM check.

v1: https://lore.kernel.org/lkml/[email protected]/


Babu Moger (4):
selftests/resctrl: Rename variables and functions to generic names
selftests/resctrl: Pass sysfs controller name of the vendor
selftests/resctrl: Add support for MBM and MBA tests on AMD
selftests/resctrl: Enable MBA/MBA tests on AMD

tools/testing/selftests/resctrl/mba_test.c | 25 +-
tools/testing/selftests/resctrl/mbm_test.c | 23 +-
tools/testing/selftests/resctrl/resctrl.h | 2 +-
tools/testing/selftests/resctrl/resctrl_val.c | 305 ++++++++++--------
tools/testing/selftests/resctrl/resctrlfs.c | 2 +-
5 files changed, 191 insertions(+), 166 deletions(-)

--
2.34.1



2024-06-05 22:47:10

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v3 3/4] selftests/resctrl: Add support for MBM and MBA tests on AMD

Add support to read UMC (Unified Memory Controller) perf events to compare
the numbers with QoS monitor for AMD.

Signed-off-by: Babu Moger <[email protected]>
---
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 {
--
2.34.1


2024-06-05 22:48:54

by Moger, Babu

[permalink] [raw]
Subject: [PATCH v3 4/4] selftests/resctrl: Enable MBA/MBA tests on AMD

Enable MBA/MBM tests if UMC (Unified Memory Controller) support is
available on the system. Tests will be skipped otherwise.

Signed-off-by: Babu Moger <[email protected]>
---
v3: Separated fix as another patch.
https://lore.kernel.org/lkml/3a6c9dd9dc6bda6e2582db049bfe853cd836139f.1717622080.git.babu.moger@amd.com/

v2: No changes.
---
tools/testing/selftests/resctrl/mba_test.c | 1 -
tools/testing/selftests/resctrl/mbm_test.c | 1 -
2 files changed, 2 deletions(-)

diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 394bbfd8a93e..5702bb1d6a98 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -192,7 +192,6 @@ static bool mba_feature_check(const struct resctrl_test *test)
struct resctrl_test mba_test = {
.name = "MBA",
.resource = "MB",
- .vendor_specific = ARCH_INTEL,
.feature_check = mba_feature_check,
.run_test = mba_run_test,
.cleanup = mba_test_cleanup,
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 9b6f7f162282..dbdb1b490df8 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -162,7 +162,6 @@ static bool mbm_feature_check(const struct resctrl_test *test)
struct resctrl_test mbm_test = {
.name = "MBM",
.resource = "MB",
- .vendor_specific = ARCH_INTEL,
.feature_check = mbm_feature_check,
.run_test = mbm_run_test,
.cleanup = mbm_test_cleanup,
--
2.34.1


2024-06-14 18:39:36

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] selftests/resctrl: Add support for MBM and MBA tests on AMD

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 <[email protected]>
> ---
> 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

2024-06-14 18:40:29

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] selftests/resctrl: Enable MBA/MBA tests on AMD

Hi Babu,

On 6/5/24 3:45 PM, Babu Moger wrote:
> Enable MBA/MBM tests if UMC (Unified Memory Controller) support is
> available on the system. Tests will be skipped otherwise.

Could you please point out where the test is skipped if UMC is
not available?

Reinette