diff mbox series

[v3,3/4] selftests/resctrl: Add support for MBM and MBA tests on AMD

Message ID 3c2034e3391634b35192819b69eabd7db8cffa8f.1717626661.git.babu.moger@amd.com (mailing list archive)
State New
Headers show
Series selftests/resctrl: Enable MBM and MBA tests on AMD | expand

Commit Message

Babu Moger June 5, 2024, 10:45 p.m. UTC
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(-)

Comments

Reinette Chatre June 14, 2024, 6:39 p.m. UTC | #1
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
Babu Moger June 20, 2024, 4:18 p.m. UTC | #2
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 mbox series

Patch

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 {