[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56b67440-f6a7-4266-a8be-1e312ebd9aae@linux.intel.com>
Date: Tue, 27 Feb 2024 19:31:59 -0800
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
To: "David E. Box" <david.e.box@...ux.intel.com>,
rajvi.jingar@...ux.intel.com, platform-driver-x86@...r.kernel.org,
linux-kernel@...r.kernel.org, hdegoede@...hat.com,
ilpo.jarvinen@...ux.intel.com
Subject: Re: [PATCH V2 9/9] tools: intel_sdsi: Add current meter support
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@...ux.intel.com>
> ---
Looks good to me.
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.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);
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Powered by blists - more mailing lists