Re: [PATCH 9/9] tools/arch/x86: intel_sdsi: Add support for reading meter certificates

Hans de Goede
2022-11-17T14:00:10Z

Hi,

On 11/1/22 20:10, David E. Box wrote:
> Add option to read and decode On Demand meter certificates.
>
> Link: https://github.com/intel/intel-sdsi/blob/master/meter-certificate.rst
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 110 ++++++++++++++++++++++++-
> 1 file changed, 107 insertions(+), 3 deletions(-)
>
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index 0680eda78b1a..ebf076ee6ef8 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -39,8 +39,10 @@
> #define GUID_V2 0xF210D9EF
> #define REGISTERS_MIN_SIZE 72
> #define STATE_CERT_MAX_SIZE 4096
> +#define METER_CERT_MAX_SIZE 4096
> #define STATE_MAX_NUM_LICENSES 16
> #define STATE_MAX_NUM_IN_BUNDLE (uint32_t)8
> +#define METER_MAX_NUM_BUNDLES 8
>
> #define __round_mask(x, y) ((__typeof__(x))((y) - 1))
> #define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1)
> @@ -150,6 +152,21 @@ struct bundle_encoding {
> uint32_t encoding_rsvd[7];
> };
>
> +struct meter_certificate {
> + uint32_t block_signature;
> + uint32_t counter_unit;
> + uint64_t ppin;
> + uint32_t bundle_length;
> + uint32_t reserved;
> + uint32_t mmrc_encoding;
> + uint32_t mmrc_counter;
> +};
> +
> +struct bundle_encoding_counter {
> + uint32_t encoding;
> + uint32_t counter;
> +};
> +
> struct sdsi_dev {
> struct sdsi_regs regs;
> struct state_certificate sc;
> @@ -160,6 +177,7 @@ struct sdsi_dev {
>
> enum command {
> CMD_SOCKET_INFO,
> + CMD_METER_CERT,
> CMD_STATE_CERT,
> CMD_PROV_AKC,
> CMD_PROV_CAP,
> @@ -306,6 +324,86 @@ static void get_feature(uint32_t encoding, char *feature)
> feature[0] = name[3];
> }
>
> +static int sdsi_meter_cert_show(struct sdsi_dev *s)
> +{
> + char buf[METER_CERT_MAX_SIZE] = {0};
> + struct bundle_encoding_counter *bec;
> + struct meter_certificate *mc;
> + uint32_t count = 0;
> + FILE *cert_ptr;
> + int ret, size;
> +
> + ret = sdsi_update_registers(s);
> + if (ret)
> + return ret;
> +
> + 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;
> + }
> +
> + if (!s->regs.en_features.metering) {
> + fprintf(stderr, "Metering not supporting on this socket.\n");
> + return -1;
> + }
> +
> + ret = chdir(s->dev_path);
> + if (ret == -1) {
> + perror("chdir");
> + return ret;
> + }
> +
> + cert_ptr = fopen("meter_certificate", "r");
> + if (!cert_ptr) {
> + perror("Could not open 'meter_certificate' file");
> + return -1;
> + }
> +
> + size = fread(buf, 1, sizeof(buf), cert_ptr);
> + if (!size) {
> + fprintf(stderr, "Could not read 'meter_certificate' file\n");
> + fclose(cert_ptr);
> + return -1;
> + }
> + fclose(cert_ptr);
> +
> + mc = (struct meter_certificate *)buf;
> +
> + printf("\n");
> + printf("Meter certificate for device %s\n", s->dev_name);
> + printf("\n");
> + printf("Block Signature: 0x%x\n", mc->block_signature);
> + printf("Count Unit: %dms\n", mc->counter_unit);
> + printf("PPIN: 0x%lx\n", mc->ppin);
> + printf("Feature Bundle Length: %d\n", mc->bundle_length);
> + printf("MMRC encoding: %d\n", mc->mmrc_encoding);
> + printf("MMRC counter: %d\n", mc->mmrc_counter);
> + if (mc->bundle_length % 8) {
> + fprintf(stderr, "Invalid bundle length\n");
> + return -1;
> + }
> +
> + if (mc->bundle_length > METER_MAX_NUM_BUNDLES * 8) {
> + fprintf(stderr, "More the %d bundles: %d\n",
> + METER_MAX_NUM_BUNDLES, mc->bundle_length / 8);
> + return -1;
> + }
> +
> + bec = (void *)(mc) + sizeof(mc);
> +
> + printf("Number of Feature Counters: %d\n", mc->bundle_length / 8);
> + while (count++ < mc->bundle_length / 8) {
> + char feature[5];
> +
> + feature[4] = '\0';
> + get_feature(bec[count].encoding, feature);
> + printf(" %s: %d\n", feature, bec[count].counter);
> + }
> +
> + return 0;
> +}
> +
> static int sdsi_state_cert_show(struct sdsi_dev *s)
> {
> char buf[STATE_CERT_MAX_SIZE] = {0};
> @@ -625,7 +723,7 @@ static void sdsi_free_dev(struct sdsi_dev *s)
>
> static void usage(char *prog)
> {
> - printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-a FILE] [-c FILE]]\n", prog);
> + printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-m] [-a FILE] [-c FILE]]\n", prog);
> }
>
> static void show_help(void)
> @@ -635,6 +733,7 @@ static void show_help(void)
> 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", "-a, --akc FILE", "provision socket with AKC FILE");
> printf(" %-18s\t%s\n", "-c, --cap FILE>", "provision socket with CAP FILE");
> }
> @@ -656,6 +755,7 @@ int main(int argc, char *argv[])
> {"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 }
> };
> @@ -663,7 +763,7 @@ int main(int argc, char *argv[])
>
> progname = argv[0];
>
> - while ((opt = getopt_long_only(argc, argv, "+a:c:d:hils", long_options,
> + while ((opt = getopt_long_only(argc, argv, "+a:c:d:hilms", long_options,
> &option_index)) != -1) {
> switch (opt) {
> case 'd':
> @@ -676,8 +776,9 @@ int main(int argc, char *argv[])
> case 'i':
> command = CMD_SOCKET_INFO;
> break;
> + case 'm':
> case 's':
> - command = CMD_STATE_CERT;
> + command = (opt == 'm') ? CMD_METER_CERT : CMD_STATE_CERT;
> break;

please just make this 2 separate cases rather then testing opt after
just having done a switch-case on opt.

Other then that this looks good to me, so with that fixed:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> case 'a':
> case 'c':
> @@ -713,6 +814,9 @@ int main(int argc, char *argv[])
> case CMD_SOCKET_INFO:
> ret = sdsi_read_reg(s);
> break;
> + case CMD_METER_CERT:
> + ret = sdsi_meter_cert_show(s);
> + break;
> case CMD_STATE_CERT:
> ret = sdsi_state_cert_show(s);
> break;