[V2,9/9] tools: intel_sdsi: Add current meter support

Message ID	20240228000016.1685518-10-david.e.box@linux.intel.com (mailing list archive)
State	Changes Requested, archived
Headers	show Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9EC382031D; Wed, 28 Feb 2024 00:00:21 +0000 (UTC) From: "David E. Box" <david.e.box@linux.intel.com> To: david.e.box@linux.intel.com, rajvi.jingar@linux.intel.com, platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, hdegoede@redhat.com, ilpo.jarvinen@linux.intel.com Subject: [PATCH V2 9/9] tools: intel_sdsi: Add current meter support Date: Tue, 27 Feb 2024 16:00:16 -0800 Message-Id: <20240228000016.1685518-10-david.e.box@linux.intel.com> In-Reply-To: <20240228000016.1685518-1-david.e.box@linux.intel.com> References: <20240228000016.1685518-1-david.e.box@linux.intel.com> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	Intel On Demand changes \| expand [V2,0/9] Intel On Demand changes [V2,1/9] platform/x86/intel/sdsi: Set message size during writes [V2,2/9] platform/x86/intel/sdsi: Combine read and write mailbox flows [V2,3/9] platform/x86/intel/sdsi: Add in-band BIOS lock support [V2,4/9] platform/x86/intel/sdsi: Add attribute to read the current meter state [V2,5/9] tools/arch/x86/intel_sdsi: Fix maximum meter bundle length [V2,6/9] tools/arch/x86/intel_sdsi: Add missing version field [V2,7/9] tools/arch/x86/intel_sdsi: Fix meter_certificate decoding [V2,8/9] platform/x86/intel/sdsi: Simplify ascii printing [V2,9/9] tools: intel_sdsi: Add current meter support

Message ID

20240228000016.1685518-10-david.e.box@linux.intel.com (mailing list archive)

State

Changes Requested, archived

Headers

From: "David E. Box" <david.e.box@linux.intel.com>
To: david.e.box@linux.intel.com,
	rajvi.jingar@linux.intel.com,
	platform-driver-x86@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	hdegoede@redhat.com,
	ilpo.jarvinen@linux.intel.com
Subject: [PATCH V2 9/9] tools: intel_sdsi: Add current meter support
Date: Tue, 27 Feb 2024 16:00:16 -0800
Message-Id: <20240228000016.1685518-10-david.e.box@linux.intel.com>
In-Reply-To: <20240228000016.1685518-1-david.e.box@linux.intel.com>
References: <20240228000016.1685518-1-david.e.box@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Series

Intel On Demand changes | expand

Commit Message

David E. Box Feb. 28, 2024, midnight UTC

Add support to read the 'meter_current' file. The display is the same as
the 'meter_certificate', but will show the current snapshot of the
counters.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---

V2 - Set the name of the file to be opened once.

 tools/arch/x86/intel_sdsi/intel_sdsi.c | 49 ++++++++++++++++----------
 1 file changed, 30 insertions(+), 19 deletions(-)

Comments

Kuppuswamy Sathyanarayanan Feb. 28, 2024, 3:31 a.m. UTC | #1

On 2/27/24 4:00 PM, David E. Box wrote:
> Add support to read the 'meter_current' file. The display is the same as
> the 'meter_certificate', but will show the current snapshot of the
> counters.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---

Looks good to me.

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> V2 - Set the name of the file to be opened once.
>
>  tools/arch/x86/intel_sdsi/intel_sdsi.c | 49 ++++++++++++++++----------
>  1 file changed, 30 insertions(+), 19 deletions(-)
>
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index a8fb6d17405f..325e1e41af1d 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -182,6 +182,7 @@ struct sdsi_dev {
>  enum command {
>  	CMD_SOCKET_INFO,
>  	CMD_METER_CERT,
> +	CMD_METER_CURRENT_CERT,
>  	CMD_STATE_CERT,
>  	CMD_PROV_AKC,
>  	CMD_PROV_CAP,
> @@ -329,13 +330,14 @@ static void get_feature(uint32_t encoding, char *feature)
>  	feature[0] = name[3];
>  }
>  
> -static int sdsi_meter_cert_show(struct sdsi_dev *s)
> +static int sdsi_meter_cert_show(struct sdsi_dev *s, bool show_current)
>  {
>  	char buf[METER_CERT_MAX_SIZE] = {0};
>  	struct bundle_encoding_counter *bec;
>  	struct meter_certificate *mc;
>  	uint32_t count = 0;
>  	FILE *cert_ptr;
> +	char *cert_fname;
Nit: I think this can be const char *
>  	int ret, size;
>  	char name[4];
>  
> @@ -345,7 +347,6 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
>  
>  	if (!s->regs.en_features.sdsi) {
>  		fprintf(stderr, "SDSi feature is present but not enabled.\n");
> -		fprintf(stderr, " Unable to read meter certificate\n");
>  		return -1;
>  	}
>  
> @@ -360,15 +361,17 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
>  		return ret;
>  	}
>  
> -	cert_ptr = fopen("meter_certificate", "r");
> +	cert_fname = show_current ? "meter_current" : "meter_certificate";
> +	cert_ptr = fopen(cert_fname, "r");
> +
>  	if (!cert_ptr) {
> -		perror("Could not open 'meter_certificate' file");
> +		fprintf(stderr, "Could not open '%s' file: %s", cert_fname, strerror(errno));
>  		return -1;
>  	}
>  
>  	size = fread(buf, 1, sizeof(buf), cert_ptr);
>  	if (!size) {
> -		fprintf(stderr, "Could not read 'meter_certificate' file\n");
> +		fprintf(stderr, "Could not read '%s' file\n", cert_fname);
>  		fclose(cert_ptr);
>  		return -1;
>  	}
> @@ -734,7 +737,7 @@ static void sdsi_free_dev(struct sdsi_dev *s)
>  
>  static void usage(char *prog)
>  {
> -	printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-m] [-a FILE] [-c FILE]]\n", prog);
> +	printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-m | -C] [-a FILE] [-c FILE]\n", prog);
>  }
>  
>  static void show_help(void)
> @@ -743,8 +746,9 @@ static void show_help(void)
>  	printf("  %-18s\t%s\n", "-l, --list",           "list available On Demand devices");
>  	printf("  %-18s\t%s\n", "-d, --devno DEVNO",    "On Demand device number");
>  	printf("  %-18s\t%s\n", "-i, --info",           "show socket information");
> -	printf("  %-18s\t%s\n", "-s, --state",          "show state certificate");
> -	printf("  %-18s\t%s\n", "-m, --meter",          "show meter certificate");
> +	printf("  %-18s\t%s\n", "-s, --state",          "show state certificate data");
> +	printf("  %-18s\t%s\n", "-m, --meter",          "show meter certificate data");
> +	printf("  %-18s\t%s\n", "-C, --meter_current",  "show live unattested meter data");
>  	printf("  %-18s\t%s\n", "-a, --akc FILE",       "provision socket with AKC FILE");
>  	printf("  %-18s\t%s\n", "-c, --cap FILE>",      "provision socket with CAP FILE");
>  }
> @@ -760,21 +764,22 @@ int main(int argc, char *argv[])
>  	int option_index = 0;
>  
>  	static struct option long_options[] = {
> -		{"akc",		required_argument,	0, 'a'},
> -		{"cap",		required_argument,	0, 'c'},
> -		{"devno",	required_argument,	0, 'd'},
> -		{"help",	no_argument,		0, 'h'},
> -		{"info",	no_argument,		0, 'i'},
> -		{"list",	no_argument,		0, 'l'},
> -		{"meter",	no_argument,		0, 'm'},
> -		{"state",	no_argument,		0, 's'},
> -		{0,		0,			0, 0 }
> +		{"akc",			required_argument,	0, 'a'},
> +		{"cap",			required_argument,	0, 'c'},
> +		{"devno",		required_argument,	0, 'd'},
> +		{"help",		no_argument,		0, 'h'},
> +		{"info",		no_argument,		0, 'i'},
> +		{"list",		no_argument,		0, 'l'},
> +		{"meter",		no_argument,		0, 'm'},
> +		{"meter_current",	no_argument,		0, 'C'},
> +		{"state",		no_argument,		0, 's'},
> +		{0,			0,			0, 0 }
>  	};
>  
>  
>  	progname = argv[0];
>  
> -	while ((opt = getopt_long_only(argc, argv, "+a:c:d:hilms", long_options,
> +	while ((opt = getopt_long_only(argc, argv, "+a:c:d:hilmCs", long_options,
>  			&option_index)) != -1) {
>  		switch (opt) {
>  		case 'd':
> @@ -790,6 +795,9 @@ int main(int argc, char *argv[])
>  		case 'm':
>  			command = CMD_METER_CERT;
>  			break;
> +		case 'C':
> +			command = CMD_METER_CURRENT_CERT;
> +			break;
>  		case 's':
>  			command = CMD_STATE_CERT;
>  			break;
> @@ -828,7 +836,10 @@ int main(int argc, char *argv[])
>  			ret = sdsi_read_reg(s);
>  			break;
>  		case CMD_METER_CERT:
> -			ret = sdsi_meter_cert_show(s);
> +			ret = sdsi_meter_cert_show(s, false);
> +			break;
> +		case CMD_METER_CURRENT_CERT:
> +			ret = sdsi_meter_cert_show(s, true);
>  			break;
>  		case CMD_STATE_CERT:
>  			ret = sdsi_state_cert_show(s);

Ilpo Järvinen March 4, 2024, 1:34 p.m. UTC | #2

On Tue, 27 Feb 2024, David E. Box wrote:

> Add support to read the 'meter_current' file. The display is the same as
> the 'meter_certificate', but will show the current snapshot of the
> counters.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> 
> V2 - Set the name of the file to be opened once.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
index a8fb6d17405f..325e1e41af1d 100644
--- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
+++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
@@ -182,6 +182,7 @@  struct sdsi_dev {
 enum command {
 	CMD_SOCKET_INFO,
 	CMD_METER_CERT,
+	CMD_METER_CURRENT_CERT,
 	CMD_STATE_CERT,
 	CMD_PROV_AKC,
 	CMD_PROV_CAP,
@@ -329,13 +330,14 @@  static void get_feature(uint32_t encoding, char *feature)
 	feature[0] = name[3];
 }
 
-static int sdsi_meter_cert_show(struct sdsi_dev *s)
+static int sdsi_meter_cert_show(struct sdsi_dev *s, bool show_current)
 {
 	char buf[METER_CERT_MAX_SIZE] = {0};
 	struct bundle_encoding_counter *bec;
 	struct meter_certificate *mc;
 	uint32_t count = 0;
 	FILE *cert_ptr;
+	char *cert_fname;
 	int ret, size;
 	char name[4];
 
@@ -345,7 +347,6 @@  static int sdsi_meter_cert_show(struct sdsi_dev *s)
 
 	if (!s->regs.en_features.sdsi) {
 		fprintf(stderr, "SDSi feature is present but not enabled.\n");
-		fprintf(stderr, " Unable to read meter certificate\n");
 		return -1;
 	}
 
@@ -360,15 +361,17 @@  static int sdsi_meter_cert_show(struct sdsi_dev *s)
 		return ret;
 	}
 
-	cert_ptr = fopen("meter_certificate", "r");
+	cert_fname = show_current ? "meter_current" : "meter_certificate";
+	cert_ptr = fopen(cert_fname, "r");
+
 	if (!cert_ptr) {
-		perror("Could not open 'meter_certificate' file");
+		fprintf(stderr, "Could not open '%s' file: %s", cert_fname, strerror(errno));
 		return -1;
 	}
 
 	size = fread(buf, 1, sizeof(buf), cert_ptr);
 	if (!size) {
-		fprintf(stderr, "Could not read 'meter_certificate' file\n");
+		fprintf(stderr, "Could not read '%s' file\n", cert_fname);
 		fclose(cert_ptr);
 		return -1;
 	}
@@ -734,7 +737,7 @@  static void sdsi_free_dev(struct sdsi_dev *s)
 
 static void usage(char *prog)
 {
-	printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-m] [-a FILE] [-c FILE]]\n", prog);
+	printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-m | -C] [-a FILE] [-c FILE]\n", prog);
 }
 
 static void show_help(void)
@@ -743,8 +746,9 @@  static void show_help(void)
 	printf("  %-18s\t%s\n", "-l, --list",           "list available On Demand devices");
 	printf("  %-18s\t%s\n", "-d, --devno DEVNO",    "On Demand device number");
 	printf("  %-18s\t%s\n", "-i, --info",           "show socket information");
-	printf("  %-18s\t%s\n", "-s, --state",          "show state certificate");
-	printf("  %-18s\t%s\n", "-m, --meter",          "show meter certificate");
+	printf("  %-18s\t%s\n", "-s, --state",          "show state certificate data");
+	printf("  %-18s\t%s\n", "-m, --meter",          "show meter certificate data");
+	printf("  %-18s\t%s\n", "-C, --meter_current",  "show live unattested meter data");
 	printf("  %-18s\t%s\n", "-a, --akc FILE",       "provision socket with AKC FILE");
 	printf("  %-18s\t%s\n", "-c, --cap FILE>",      "provision socket with CAP FILE");
 }
@@ -760,21 +764,22 @@  int main(int argc, char *argv[])
 	int option_index = 0;
 
 	static struct option long_options[] = {
-		{"akc",		required_argument,	0, 'a'},
-		{"cap",		required_argument,	0, 'c'},
-		{"devno",	required_argument,	0, 'd'},
-		{"help",	no_argument,		0, 'h'},
-		{"info",	no_argument,		0, 'i'},
-		{"list",	no_argument,		0, 'l'},
-		{"meter",	no_argument,		0, 'm'},
-		{"state",	no_argument,		0, 's'},
-		{0,		0,			0, 0 }
+		{"akc",			required_argument,	0, 'a'},
+		{"cap",			required_argument,	0, 'c'},
+		{"devno",		required_argument,	0, 'd'},
+		{"help",		no_argument,		0, 'h'},
+		{"info",		no_argument,		0, 'i'},
+		{"list",		no_argument,		0, 'l'},
+		{"meter",		no_argument,		0, 'm'},
+		{"meter_current",	no_argument,		0, 'C'},
+		{"state",		no_argument,		0, 's'},
+		{0,			0,			0, 0 }
 	};
 
 
 	progname = argv[0];
 
-	while ((opt = getopt_long_only(argc, argv, "+a:c:d:hilms", long_options,
+	while ((opt = getopt_long_only(argc, argv, "+a:c:d:hilmCs", long_options,
 			&option_index)) != -1) {
 		switch (opt) {
 		case 'd':
@@ -790,6 +795,9 @@  int main(int argc, char *argv[])
 		case 'm':
 			command = CMD_METER_CERT;
 			break;
+		case 'C':
+			command = CMD_METER_CURRENT_CERT;
+			break;
 		case 's':
 			command = CMD_STATE_CERT;
 			break;
@@ -828,7 +836,10 @@  int main(int argc, char *argv[])
 			ret = sdsi_read_reg(s);
 			break;
 		case CMD_METER_CERT:
-			ret = sdsi_meter_cert_show(s);
+			ret = sdsi_meter_cert_show(s, false);
+			break;
+		case CMD_METER_CURRENT_CERT:
+			ret = sdsi_meter_cert_show(s, true);
 			break;
 		case CMD_STATE_CERT:
 			ret = sdsi_state_cert_show(s);

[V2,9/9] tools: intel_sdsi: Add current meter support

Commit Message

Comments

Patch