Intel Software Defined Silicon (SDSi) is now known as Intel On Demand. The
following patches do the following:
1. Identify the driver/tools as Intel On Demand. Only text descriptions are
changed. Kconfig and filenames remain the same.
2. Perform some attribute cleanup by preventing the showing of files when
features are not supported.
3. Adds support for a new GUID. GUIDs are used to identify the layout of
the On Demand registers in sysfs. Layouts are described in the
documentation on github [1].
4. Add support for reading On Demand meter certificates in sysfs.
5. The rest of the patches modify the existing tool to support discovery
and reading of On Demand registers and the meter certificate.
[1] https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
David E. Box (9):
platform/x86/intel/sdsi: Add Intel On Demand text
platform/x86/intel/sdsi: Hide attributes if hardware doesn't support
platform/x86/intel/sdsi: Support different GUIDs
platform/x86/intel/sdsi: Add meter certificate support
tools/arch/x86: intel_sdsi: Add support for reading state certificates
tools/arch/x86: intel_sdsi: Add Intel On Demand text
tools/arch/x86: intel_sdsi: Read more On Demand registers
tools/arch/x86: intel_sdsi: Add support for new GUID
tools/arch/x86: intel_sdsi: Add support for reading meter certificates
.../ABI/testing/sysfs-driver-intel_sdsi | 47 +-
drivers/platform/x86/intel/Kconfig | 8 +-
drivers/platform/x86/intel/sdsi.c | 134 ++++-
tools/arch/x86/intel_sdsi/intel_sdsi.c | 458 ++++++++++++++----
4 files changed, 513 insertions(+), 134 deletions(-)
base-commit: 225469d4acbcb873358d7618bad6e0203b67b964
--
2.25.1
Newer versions of Intel On Demand hardware may have an expanded list of
registers to support new features. The register layout is identified by a
unique GUID that's read during driver probe. Add support for handling
different GUIDs and add support for current GUIDs [1].
[1] https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
Signed-off-by: David E. Box <[email protected]>
---
drivers/platform/x86/intel/sdsi.c | 47 +++++++++++++++++++++++++++++--
1 file changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index bca05b4dd983..ab1f65919fc5 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -27,10 +27,10 @@
#define ACCESS_TYPE_LOCAL 3
#define SDSI_MIN_SIZE_DWORDS 276
-#define SDSI_SIZE_CONTROL 8
#define SDSI_SIZE_MAILBOX 1024
-#define SDSI_SIZE_REGS 72
+#define SDSI_SIZE_REGS 80
#define SDSI_SIZE_CMD sizeof(u64)
+#define SDSI_SIZE_MAILBOX 1024
/*
* Write messages are currently up to the size of the mailbox
@@ -76,6 +76,9 @@
#define DT_TBIR GENMASK(2, 0)
#define DT_OFFSET(v) ((v) & GENMASK(31, 3))
+#define SDSI_GUID_V1 0x006DD191
+#define SDSI_GUID_V2 0xF210D9EF
+
enum sdsi_command {
SDSI_CMD_PROVISION_AKC = 0x04,
SDSI_CMD_PROVISION_CAP = 0x08,
@@ -100,6 +103,9 @@ struct sdsi_priv {
void __iomem *control_addr;
void __iomem *mbox_addr;
void __iomem *regs_addr;
+ int control_size;
+ int maibox_size;
+ int registers_size;
u32 guid;
u32 features;
};
@@ -444,6 +450,18 @@ static ssize_t registers_read(struct file *filp, struct kobject *kobj,
struct device *dev = kobj_to_dev(kobj);
struct sdsi_priv *priv = dev_get_drvdata(dev);
void __iomem *addr = priv->regs_addr;
+ int size = priv->registers_size;
+
+ /*
+ * The check below is performed by the sysfs caller based on the static
+ * file size. But this may be greater than the actual size which is based
+ * on the GUID. So check here again based on actual size before reading.
+ */
+ if (off >= size)
+ return 0;
+
+ if (off + count > size)
+ count = size - off;
memcpy_fromio(buf, addr + off, count);
@@ -496,6 +514,24 @@ static const struct attribute_group sdsi_group = {
};
__ATTRIBUTE_GROUPS(sdsi);
+static int sdsi_get_layout(struct sdsi_priv *priv, struct disc_table *table)
+{
+ switch (table->guid) {
+ case SDSI_GUID_V1:
+ priv->control_size = 8;
+ priv->registers_size = 72;
+ break;
+ case SDSI_GUID_V2:
+ priv->control_size = 16;
+ priv->registers_size = 80;
+ break;
+ default:
+ dev_err(priv->dev, "Unrecognized GUID 0x%x\n", table->guid);
+ return -EINVAL;
+ }
+ return 0;
+}
+
static int sdsi_map_mbox_registers(struct sdsi_priv *priv, struct pci_dev *parent,
struct disc_table *disc_table, struct resource *disc_res)
{
@@ -537,7 +573,7 @@ static int sdsi_map_mbox_registers(struct sdsi_priv *priv, struct pci_dev *paren
if (IS_ERR(priv->control_addr))
return PTR_ERR(priv->control_addr);
- priv->mbox_addr = priv->control_addr + SDSI_SIZE_CONTROL;
+ priv->mbox_addr = priv->control_addr + priv->control_size;
priv->regs_addr = priv->mbox_addr + SDSI_SIZE_MAILBOX;
priv->features = readq(priv->regs_addr + SDSI_ENABLED_FEATURES_OFFSET);
@@ -572,6 +608,11 @@ static int sdsi_probe(struct auxiliary_device *auxdev, const struct auxiliary_de
priv->guid = disc_table.guid;
+ /* Get guid based layout info */
+ ret = sdsi_get_layout(priv, &disc_table);
+ if (ret)
+ return ret;
+
/* Map the SDSi mailbox registers */
ret = sdsi_map_mbox_registers(priv, intel_cap_dev->pcidev, &disc_table, disc_res);
if (ret)
--
2.25.1
Intel Software Defined Silicon (SDSi) is now officially known as Intel
On Demand. Change text in tool to indicate this.
Signed-off-by: David E. Box <[email protected]>
---
tools/arch/x86/intel_sdsi/intel_sdsi.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
index 9dd94014a672..3718bd0c05cb 100644
--- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
+++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * sdsi: Intel Software Defined Silicon tool for provisioning certificates
- * and activation payloads on supported cpus.
+ * sdsi: Intel On Demand (formerly Software Defined Silicon) tool for
+ * provisioning certificates and activation payloads on supported cpus.
*
* See https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
* for register descriptions.
@@ -150,7 +150,7 @@ static void sdsi_list_devices(void)
}
if (!found)
- fprintf(stderr, "No sdsi devices found.\n");
+ fprintf(stderr, "No On Demand devices found.\n");
}
static int sdsi_update_registers(struct sdsi_dev *s)
@@ -206,7 +206,7 @@ static int sdsi_read_reg(struct sdsi_dev *s)
printf("\n");
printf("PPIN: 0x%lx\n", s->regs.ppin);
printf("Enabled Features\n");
- printf(" SDSi: %s\n", !!s->regs.en_features.sdsi ? "Enabled" : "Disabled");
+ printf(" On Demand: %s\n", !!s->regs.en_features.sdsi ? "Enabled" : "Disabled");
printf("Authorization Failure Count\n");
printf(" AKC Failure Count: %d\n", s->regs.auth_fail_count.key_failure_count);
printf(" AKC Failure Threshold: %d\n", s->regs.auth_fail_count.key_failure_threshold);
@@ -428,7 +428,7 @@ static int sdsi_provision_akc(struct sdsi_dev *s, char *bin_file)
return ret;
if (!s->regs.en_features.sdsi) {
- fprintf(stderr, "SDSi feature is present but not enabled. Unable to provision");
+ fprintf(stderr, "On Demand feature is present but not enabled. Unable to provision");
return -1;
}
@@ -458,7 +458,7 @@ static int sdsi_provision_cap(struct sdsi_dev *s, char *bin_file)
return ret;
if (!s->regs.en_features.sdsi) {
- fprintf(stderr, "SDSi feature is present but not enabled. Unable to provision");
+ fprintf(stderr, "On Demand feature is present but not enabled. Unable to provision");
return -1;
}
--
2.25.1
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 <[email protected]>
---
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;
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;
--
2.25.1
Intel Software Defined Silicon (SDSi) is now officially known as Intel
On Demand. Add On Demand to the description in the kconfig, documentation,
and driver source.
Signed-off-by: David E. Box <[email protected]>
---
.../ABI/testing/sysfs-driver-intel_sdsi | 37 ++++++++++---------
drivers/platform/x86/intel/Kconfig | 8 ++--
drivers/platform/x86/intel/sdsi.c | 4 +-
3 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel_sdsi b/Documentation/ABI/testing/sysfs-driver-intel_sdsi
index 96b92c105ec4..9d77f30d9b9a 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel_sdsi
+++ b/Documentation/ABI/testing/sysfs-driver-intel_sdsi
@@ -4,21 +4,21 @@ KernelVersion: 5.18
Contact: "David E. Box" <[email protected]>
Description:
This directory contains interface files for accessing Intel
- Software Defined Silicon (SDSi) features on a CPU. X
- represents the socket instance (though not the socket ID).
- The socket ID is determined by reading the registers file
- and decoding it per the specification.
+ On Demand (formerly Software Defined Silicon or SDSi) features
+ on a CPU. X represents the socket instance (though not the
+ socket ID). The socket ID is determined by reading the
+ registers file and decoding it per the specification.
- Some files communicate with SDSi hardware through a mailbox.
- Should the operation fail, one of the following error codes
- may be returned:
+ Some files communicate with On Demand hardware through a
+ mailbox. Should the operation fail, one of the following error
+ codes may be returned:
========== =====
Error Code Cause
========== =====
EIO General mailbox failure. Log may indicate cause.
EBUSY Mailbox is owned by another agent.
- EPERM SDSI capability is not enabled in hardware.
+ EPERM On Demand capability is not enabled in hardware.
EPROTO Failure in mailbox protocol detected by driver.
See log for details.
EOVERFLOW For provision commands, the size of the data
@@ -54,8 +54,8 @@ KernelVersion: 5.18
Contact: "David E. Box" <[email protected]>
Description:
(WO) Used to write an Authentication Key Certificate (AKC) to
- the SDSi NVRAM for the CPU. The AKC is used to authenticate a
- Capability Activation Payload. Mailbox command.
+ the On Demand NVRAM for the CPU. The AKC is used to authenticate
+ a Capability Activation Payload. Mailbox command.
What: /sys/bus/auxiliary/devices/intel_vsec.sdsi.X/provision_cap
Date: Feb 2022
@@ -63,17 +63,18 @@ KernelVersion: 5.18
Contact: "David E. Box" <[email protected]>
Description:
(WO) Used to write a Capability Activation Payload (CAP) to the
- SDSi NVRAM for the CPU. CAPs are used to activate a given CPU
- feature. A CAP is validated by SDSi hardware using a previously
- provisioned AKC file. Upon successful authentication, the CPU
- configuration is updated. A cold reboot is required to fully
- activate the feature. Mailbox command.
+ On Demand NVRAM for the CPU. CAPs are used to activate a given
+ CPU feature. A CAP is validated by On Demand hardware using a
+ previously provisioned AKC file. Upon successful authentication,
+ the CPU configuration is updated. A cold reboot is required to
+ fully activate the feature. Mailbox command.
What: /sys/bus/auxiliary/devices/intel_vsec.sdsi.X/state_certificate
Date: Feb 2022
KernelVersion: 5.18
Contact: "David E. Box" <[email protected]>
Description:
- (RO) Used to read back the current State Certificate for the CPU
- from SDSi hardware. The State Certificate contains information
- about the current licenses on the CPU. Mailbox command.
+ (RO) Used to read back the current state certificate for the CPU
+ from On Demand hardware. The state certificate contains
+ information about the current licenses on the CPU. Mailbox
+ command.
diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig
index 794968bda115..d5a33473e838 100644
--- a/drivers/platform/x86/intel/Kconfig
+++ b/drivers/platform/x86/intel/Kconfig
@@ -157,13 +157,13 @@ config INTEL_RST
as usual.
config INTEL_SDSI
- tristate "Intel Software Defined Silicon Driver"
+ tristate "Intel On Demand (Software Defined Silicon) Driver"
depends on INTEL_VSEC
depends on X86_64
help
- This driver enables access to the Intel Software Defined Silicon
- interface used to provision silicon features with an authentication
- certificate and capability license.
+ This driver enables access to the Intel On Demand (formerly Software
+ Defined Silicon) interface used to provision silicon features with an
+ authentication certificate and capability license.
To compile this driver as a module, choose M here: the module will
be called intel_sdsi.
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index c830e98dfa38..32793919473d 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Intel Software Defined Silicon driver
+ * Intel On Demand (Software Defined Silicon) driver
*
* Copyright (c) 2022, Intel Corporation.
* All Rights Reserved.
@@ -586,5 +586,5 @@ static struct auxiliary_driver sdsi_aux_driver = {
module_auxiliary_driver(sdsi_aux_driver);
MODULE_AUTHOR("David E. Box <[email protected]>");
-MODULE_DESCRIPTION("Intel Software Defined Silicon driver");
+MODULE_DESCRIPTION("Intel On Demand (SDSi) driver");
MODULE_LICENSE("GPL");
--
2.25.1
The structure and content of the On Demand registers is based on the GUID
which is read from hardware through sysfs. Add support for decoding the
registers of a new GUID 0xF210D9EF.
Signed-off-by: David E. Box <[email protected]>
---
tools/arch/x86/intel_sdsi/intel_sdsi.c | 32 ++++++++++++++++++--------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
index 01b5f9994e11..0680eda78b1a 100644
--- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
+++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
@@ -35,7 +35,8 @@
#define SDSI_DEV "intel_vsec.sdsi"
#define AUX_DEV_PATH "/sys/bus/auxiliary/devices/"
#define SDSI_PATH (AUX_DEV_DIR SDSI_DEV)
-#define GUID 0x6dd191
+#define GUID_V1 0x6dd191
+#define GUID_V2 0xF210D9EF
#define REGISTERS_MIN_SIZE 72
#define STATE_CERT_MAX_SIZE 4096
#define STATE_MAX_NUM_LICENSES 16
@@ -100,9 +101,17 @@ struct sdsi_regs {
struct availability prov_avail;
struct nvram_update_limit limits;
uint64_t pcu_cr3_capid_cfg;
- uint64_t socket_id;
+ union {
+ struct {
+ uint64_t socket_id;
+ } v1;
+ struct {
+ uint64_t reserved;
+ uint64_t socket_id;
+ uint64_t reserved2;
+ } v2;
+ } extra;
};
-
#define CONTENT_TYPE_LK_ENC 0xD
#define CONTENT_TYPE_LK_BLOB_ENC 0xE
@@ -146,7 +155,7 @@ struct sdsi_dev {
struct state_certificate sc;
char *dev_name;
char *dev_path;
- int guid;
+ uint32_t guid;
};
enum command {
@@ -199,7 +208,7 @@ static int sdsi_update_registers(struct sdsi_dev *s)
return -1;
}
- if (s->guid != GUID) {
+ if (s->guid != GUID_V1 && s->guid != GUID_V2) {
fprintf(stderr, "Unrecognized guid, 0x%x\n", s->guid);
fclose(regs_ptr);
return -1;
@@ -207,7 +216,7 @@ static int sdsi_update_registers(struct sdsi_dev *s)
/* Update register info for this guid */
ret = fread(&s->regs, sizeof(uint8_t), sizeof(s->regs), regs_ptr);
- if (ret != sizeof(s->regs)) {
+ if (ret > (int)sizeof(s->regs)) { /* FIXME: Check size by guid */
fprintf(stderr, "Could not read 'registers' file\n");
fclose(regs_ptr);
return -1;
@@ -252,10 +261,13 @@ static int sdsi_read_reg(struct sdsi_dev *s)
printf(" Updates Available: %d\n", s->regs.prov_avail.available);
printf(" Updates Threshold: %d\n", s->regs.prov_avail.threshold);
printf("NVRAM Udate Limit\n");
- printf(" 50%% Limit Reached: %s\n", !!s->regs.limits.sdsi_50_pct ? "Yes" : "No");
- printf(" 75%% Limit Reached: %s\n", !!s->regs.limits.sdsi_75_pct ? "Yes" : "No");
- printf(" 90%% Limit Reached: %s\n", !!s->regs.limits.sdsi_90_pct ? "Yes" : "No");
- printf("Socket ID: %ld\n", s->regs.socket_id & 0xF);
+ printf(" 50%% Limit Reached: %s\n", !!s->regs.limits.sdsi_50_pct ? "Yes" : "No");
+ printf(" 75%% Limit Reached: %s\n", !!s->regs.limits.sdsi_75_pct ? "Yes" : "No");
+ printf(" 90%% Limit Reached: %s\n", !!s->regs.limits.sdsi_90_pct ? "Yes" : "No");
+ if (s->guid == GUID_V1)
+ printf("Socket ID: %ld\n", s->regs.extra.v1.socket_id & 0xF);
+ else
+ printf("Socket ID: %ld\n", s->regs.extra.v2.socket_id & 0xF);
return 0;
}
--
2.25.1
Add support for reading the meter certificate from Intel On Demand
hardware. The meter certificate [1] is used to access the utilization
metrics of enabled features in support of the Intel On Demand consumption
model. Similar to the state certificate, the meter certificate is read by
mailbox command.
[1] https://github.com/intel-sandbox/debox1.intel_sdsi/blob/gnr-review/meter-certificate.rst
Signed-off-by: David E. Box <[email protected]>
---
.../ABI/testing/sysfs-driver-intel_sdsi | 10 ++++
drivers/platform/x86/intel/sdsi.c | 52 +++++++++++++++----
2 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-intel_sdsi b/Documentation/ABI/testing/sysfs-driver-intel_sdsi
index 9d77f30d9b9a..f8afed127107 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel_sdsi
+++ b/Documentation/ABI/testing/sysfs-driver-intel_sdsi
@@ -69,6 +69,16 @@ Description:
the CPU configuration is updated. A cold reboot is required to
fully activate the feature. Mailbox command.
+What: /sys/bus/auxiliary/devices/intel_vsec.sdsi.X/meter_certificate
+Date: Nov 2022
+KernelVersion: 6.2
+Contact: "David E. Box" <[email protected]>
+Description:
+ (RO) Used to read back the current meter certificate for the CPU
+ from Intel On Demand hardware. The meter certificate contains
+ utilization metrics of On Demand enabled features. Mailbox
+ command.
+
What: /sys/bus/auxiliary/devices/intel_vsec.sdsi.X/state_certificate
Date: Feb 2022
KernelVersion: 5.18
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index ab1f65919fc5..1dee10822df7 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -42,6 +42,7 @@
#define SDSI_ENABLED_FEATURES_OFFSET 16
#define SDSI_FEATURE_SDSI BIT(3)
+#define SDSI_FEATURE_METERING BIT(26)
#define SDSI_SOCKET_ID_OFFSET 64
#define SDSI_SOCKET_ID GENMASK(3, 0)
@@ -80,9 +81,10 @@
#define SDSI_GUID_V2 0xF210D9EF
enum sdsi_command {
- SDSI_CMD_PROVISION_AKC = 0x04,
- SDSI_CMD_PROVISION_CAP = 0x08,
- SDSI_CMD_READ_STATE = 0x10,
+ SDSI_CMD_PROVISION_AKC = 0x0004,
+ SDSI_CMD_PROVISION_CAP = 0x0008,
+ SDSI_CMD_READ_STATE = 0x0010,
+ SDSI_CMD_READ_METER = 0x0014,
};
struct sdsi_mbox_info {
@@ -398,13 +400,10 @@ static ssize_t provision_cap_write(struct file *filp, struct kobject *kobj,
}
static BIN_ATTR_WO(provision_cap, SDSI_SIZE_WRITE_MSG);
-static long state_certificate_read(struct file *filp, struct kobject *kobj,
- struct bin_attribute *attr, char *buf, loff_t off,
- size_t count)
+static ssize_t
+certificate_read(u64 command, struct sdsi_priv *priv, char *buf, loff_t off,
+ size_t count)
{
- struct device *dev = kobj_to_dev(kobj);
- struct sdsi_priv *priv = dev_get_drvdata(dev);
- u64 command = SDSI_CMD_READ_STATE;
struct sdsi_mbox_info info;
size_t size;
int ret;
@@ -441,8 +440,31 @@ static long state_certificate_read(struct file *filp, struct kobject *kobj,
return size;
}
+
+static ssize_t
+state_certificate_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t off,
+ size_t count)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct sdsi_priv *priv = dev_get_drvdata(dev);
+
+ return certificate_read(SDSI_CMD_READ_STATE, priv, buf, off, count);
+}
static BIN_ATTR(state_certificate, 0400, state_certificate_read, NULL, SDSI_SIZE_READ_MSG);
+static ssize_t
+meter_certificate_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t off,
+ size_t count)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct sdsi_priv *priv = dev_get_drvdata(dev);
+
+ return certificate_read(SDSI_CMD_READ_METER, priv, buf, off, count);
+}
+static BIN_ATTR(meter_certificate, 0400, meter_certificate_read, NULL, SDSI_SIZE_READ_MSG);
+
static ssize_t registers_read(struct file *filp, struct kobject *kobj,
struct bin_attribute *attr, char *buf, loff_t off,
size_t count)
@@ -472,6 +494,7 @@ static BIN_ATTR(registers, 0400, registers_read, NULL, SDSI_SIZE_REGS);
static struct bin_attribute *sdsi_bin_attrs[] = {
&bin_attr_registers,
&bin_attr_state_certificate,
+ &bin_attr_meter_certificate,
&bin_attr_provision_akc,
&bin_attr_provision_cap,
NULL
@@ -491,7 +514,16 @@ sdsi_battr_is_visible(struct kobject *kobj, struct bin_attribute *attr, int n)
if (!(priv->features & SDSI_FEATURE_SDSI))
return 0;
- return attr->attr.mode;
+ if (attr == &bin_attr_state_certificate ||
+ attr == &bin_attr_provision_akc ||
+ attr == &bin_attr_provision_cap)
+ return attr->attr.mode;
+
+ if (attr == &bin_attr_meter_certificate &&
+ !!(priv->features & SDSI_FEATURE_METERING))
+ return attr->attr.mode;
+
+ return 0;
}
static ssize_t guid_show(struct device *dev, struct device_attribute *attr, char *buf)
--
2.25.1
Add option to read and decode On Demand state certificates.
Link: https://github.com/intel/intel-sdsi/blob/master/state-certificate-encoding.rst
Signed-off-by: David E. Box <[email protected]>
---
tools/arch/x86/intel_sdsi/intel_sdsi.c | 268 ++++++++++++++++++-------
1 file changed, 198 insertions(+), 70 deletions(-)
diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
index c0e2f2349db4..9dd94014a672 100644
--- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
+++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
@@ -22,11 +22,24 @@
#include <sys/types.h>
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+
+#define min(x, y) ({ \
+ typeof(x) _min1 = (x); \
+ typeof(y) _min2 = (y); \
+ (void) (&_min1 == &_min2); \
+ _min1 < _min2 ? _min1 : _min2; })
+
#define SDSI_DEV "intel_vsec.sdsi"
#define AUX_DEV_PATH "/sys/bus/auxiliary/devices/"
#define SDSI_PATH (AUX_DEV_DIR SDSI_DEV)
#define GUID 0x6dd191
#define REGISTERS_MIN_SIZE 72
+#define STATE_CERT_MAX_SIZE 4096
+#define STATE_MAX_NUM_LICENSES 16
+#define STATE_MAX_NUM_IN_BUNDLE (uint32_t)8
#define __round_mask(x, y) ((__typeof__(x))((y) - 1))
#define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1)
@@ -49,6 +62,7 @@ struct availability {
uint64_t reserved:48;
uint64_t available:3;
uint64_t threshold:3;
+ uint64_t reserved2:10;
};
struct sdsi_regs {
@@ -63,17 +77,55 @@ struct sdsi_regs {
uint64_t socket_id;
};
+#define CONTENT_TYPE_LK_ENC 0xD
+#define CONTENT_TYPE_LK_BLOB_ENC 0xE
+
+struct state_certificate {
+ uint32_t content_type;
+ uint32_t region_rev_id;
+ uint32_t header_size;
+ uint32_t total_size;
+ uint32_t key_size;
+ uint32_t num_licenses;
+};
+
+struct license_key_info {
+ uint32_t key_rev_id;
+ uint64_t key_image_content[6];
+} __packed;
+
+#define LICENSE_BLOB_SIZE(l) (((l) & 0x7fffffff) * 4)
+#define LICENSE_VALID(l) (!!((l) & 0x80000000))
+
+// License Group Types
+#define LBT_ONE_TIME_UPGRADE 1
+#define LBT_METERED_UPGRADE 2
+
+struct license_blob_content {
+ uint32_t type;
+ uint64_t id;
+ uint64_t ppin;
+ uint64_t previous_ppin;
+ uint32_t rev_id;
+ uint32_t num_bundles;
+} __packed;
+
+struct bundle_encoding {
+ uint32_t encoding;
+ uint32_t encoding_rsvd[7];
+};
+
struct sdsi_dev {
struct sdsi_regs regs;
+ struct state_certificate sc;
char *dev_name;
char *dev_path;
int guid;
};
enum command {
- CMD_NONE,
CMD_SOCKET_INFO,
- CMD_DUMP_CERT,
+ CMD_STATE_CERT,
CMD_PROV_AKC,
CMD_PROV_CAP,
};
@@ -168,20 +220,56 @@ static int sdsi_read_reg(struct sdsi_dev *s)
return 0;
}
-static int sdsi_certificate_dump(struct sdsi_dev *s)
+static char *license_blob_type(uint32_t type)
+{
+ switch (type) {
+ case LBT_ONE_TIME_UPGRADE:
+ return "One time upgrade";
+ case LBT_METERED_UPGRADE:
+ return "Metered upgrade";
+ default:
+ return "Unknown license blob type";
+ }
+}
+
+static char *content_type(uint32_t type)
+{
+ switch (type) {
+ case CONTENT_TYPE_LK_ENC:
+ return "Licencse key encoding";
+ case CONTENT_TYPE_LK_BLOB_ENC:
+ return "License key + Blob encoding";
+ default:
+ return "Unknown content type";
+ }
+}
+
+static void get_feature(uint32_t encoding, char *feature)
+{
+ char *name = (char *)&encoding;
+
+ feature[3] = name[0];
+ feature[2] = name[1];
+ feature[1] = name[2];
+ feature[0] = name[3];
+}
+
+static int sdsi_state_cert_show(struct sdsi_dev *s)
{
- uint64_t state_certificate[512] = {0};
- bool first_instance;
- uint64_t previous;
+ char buf[STATE_CERT_MAX_SIZE] = {0};
+ struct state_certificate *sc;
+ struct license_key_info *lki;
+ uint32_t offset = 0;
+ uint32_t count = 0;
FILE *cert_ptr;
- int i, ret, size;
+ 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.");
+ fprintf(stderr, "On Demand feature is present but not enabled.");
fprintf(stderr, " Unable to read state certificate");
return -1;
}
@@ -198,32 +286,74 @@ static int sdsi_certificate_dump(struct sdsi_dev *s)
return -1;
}
- size = fread(state_certificate, 1, sizeof(state_certificate), cert_ptr);
+ size = fread(buf, 1, sizeof(buf), cert_ptr);
if (!size) {
fprintf(stderr, "Could not read 'state_certificate' file\n");
fclose(cert_ptr);
return -1;
}
+ fclose(cert_ptr);
- printf("%3d: 0x%lx\n", 0, state_certificate[0]);
- previous = state_certificate[0];
- first_instance = true;
+ sc = (struct state_certificate *)buf;
- for (i = 1; i < (int)(round_up(size, sizeof(uint64_t))/sizeof(uint64_t)); i++) {
- if (state_certificate[i] == previous) {
- if (first_instance) {
- puts("*");
- first_instance = false;
- }
- continue;
+ /* Print register info for this guid */
+ printf("\n");
+ printf("State certificate for device %s\n", s->dev_name);
+ printf("\n");
+ printf("Content Type: %s\n", content_type(sc->content_type));
+ printf("Region Revision ID: %d\n", sc->region_rev_id);
+ printf("Header Size: %d\n", sc->header_size * 4);
+ printf("Total Size: %d\n", sc->total_size);
+ printf("OEM Key Size: %d\n", sc->key_size * 4);
+ printf("Number of Licenses: %d\n", sc->num_licenses);
+
+ /* Skip over the license sizes 4 bytes per license) to get the license key info */
+ lki = (void *)sc + sizeof(*sc) + (4 * sc->num_licenses);
+
+ printf("License blob Info:\n");
+ printf(" License Key Revision ID: 0x%x\n", lki->key_rev_id);
+ printf(" License Key Image Content: 0x%lx%lx%lx%lx%lx%lx\n",
+ lki->key_image_content[5], lki->key_image_content[4],
+ lki->key_image_content[3], lki->key_image_content[2],
+ lki->key_image_content[1], lki->key_image_content[0]);
+
+ while (count++ < sc->num_licenses) {
+ uint32_t blob_size_field = *(uint32_t *)(buf + 0x14 + count * 4);
+ uint32_t blob_size = LICENSE_BLOB_SIZE(blob_size_field);
+ bool license_valid = LICENSE_VALID(blob_size_field);
+ struct license_blob_content *lbc =
+ (void *)(sc) + // start of the state certificate
+ sizeof(*sc) + // size of the state certificate
+ (4 * sc->num_licenses) + // total size of the blob size blocks
+ sizeof(*lki) + // size of the license key info
+ offset; // offset to this blob content
+ struct bundle_encoding *bundle = (void *)(lbc) + sizeof(*lbc);
+ char feature[5];
+ uint32_t i;
+
+ printf(" Blob %d:\n", count - 1);
+ printf(" License blob size: %u\n", blob_size);
+ printf(" License is valid: %s\n", license_valid ? "Yes" : "No");
+ printf(" License blob type: %s\n", license_blob_type(lbc->type));
+ printf(" License blob ID: 0x%lx\n", lbc->id);
+ printf(" PPIN: 0x%lx\n", lbc->ppin);
+ printf(" Previous PPIN: 0x%lx\n", lbc->previous_ppin);
+ printf(" Blob revision ID: %u\n", lbc->rev_id);
+ printf(" Number of Features: %u\n", lbc->num_bundles);
+
+ feature[4] = '\0';
+
+ for (i = 0; i < min(lbc->num_bundles, STATE_MAX_NUM_IN_BUNDLE); i++) {
+ get_feature(bundle[i].encoding, feature);
+ printf(" Feature %d: %s\n", i, feature);
}
- printf("%3d: 0x%lx\n", i, state_certificate[i]);
- previous = state_certificate[i];
- first_instance = true;
- }
- printf("%3d\n", i);
- fclose(cert_ptr);
+ if (lbc->num_bundles > STATE_MAX_NUM_IN_BUNDLE)
+ fprintf(stderr, " Warning: %d > %d licenses in bundle reported.\n",
+ lbc->num_bundles, STATE_MAX_NUM_IN_BUNDLE);
+
+ offset += blob_size;
+ };
return 0;
}
@@ -231,7 +361,7 @@ static int sdsi_certificate_dump(struct sdsi_dev *s)
static int sdsi_provision(struct sdsi_dev *s, char *bin_file, enum command command)
{
int bin_fd, prov_fd, size, ret;
- char buf[4096] = { 0 };
+ char buf[STATE_CERT_MAX_SIZE] = { 0 };
char cap[] = "provision_cap";
char akc[] = "provision_akc";
char *prov_file;
@@ -266,7 +396,7 @@ static int sdsi_provision(struct sdsi_dev *s, char *bin_file, enum command comma
}
/* Read the binary file into the buffer */
- size = read(bin_fd, buf, 4096);
+ size = read(bin_fd, buf, STATE_CERT_MAX_SIZE);
if (size == -1) {
close(bin_fd);
close(prov_fd);
@@ -443,25 +573,26 @@ static void sdsi_free_dev(struct sdsi_dev *s)
static void usage(char *prog)
{
- printf("Usage: %s [-l] [-d DEVNO [-iD] [-a FILE] [-c FILE]]\n", prog);
+ printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-a FILE] [-c FILE]]\n", prog);
}
static void show_help(void)
{
printf("Commands:\n");
- printf(" %-18s\t%s\n", "-l, --list", "list available sdsi devices");
- printf(" %-18s\t%s\n", "-d, --devno DEVNO", "sdsi device number");
- printf(" %-18s\t%s\n", "-i --info", "show socket information");
- printf(" %-18s\t%s\n", "-D --dump", "dump state certificate 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");
+ 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", "-a, --akc FILE", "provision socket with AKC FILE");
+ printf(" %-18s\t%s\n", "-c, --cap FILE>", "provision socket with CAP FILE");
}
int main(int argc, char *argv[])
{
char bin_file[PATH_MAX], *dev_no = NULL;
+ bool device_selected = false;
char *progname;
- enum command command = CMD_NONE;
+ enum command command = -1;
struct sdsi_dev *s;
int ret = 0, opt;
int option_index = 0;
@@ -470,21 +601,22 @@ int main(int argc, char *argv[])
{"akc", required_argument, 0, 'a'},
{"cap", required_argument, 0, 'c'},
{"devno", required_argument, 0, 'd'},
- {"dump", no_argument, 0, 'D'},
{"help", no_argument, 0, 'h'},
{"info", no_argument, 0, 'i'},
{"list", no_argument, 0, 'l'},
+ {"state", no_argument, 0, 's'},
{0, 0, 0, 0 }
};
progname = argv[0];
- while ((opt = getopt_long_only(argc, argv, "+a:c:d:Da:c:h", long_options,
+ while ((opt = getopt_long_only(argc, argv, "+a:c:d:hils", long_options,
&option_index)) != -1) {
switch (opt) {
case 'd':
dev_no = optarg;
+ device_selected = true;
break;
case 'l':
sdsi_list_devices();
@@ -492,8 +624,8 @@ int main(int argc, char *argv[])
case 'i':
command = CMD_SOCKET_INFO;
break;
- case 'D':
- command = CMD_DUMP_CERT;
+ case 's':
+ command = CMD_STATE_CERT;
break;
case 'a':
case 'c':
@@ -520,39 +652,35 @@ int main(int argc, char *argv[])
}
}
- if (!dev_no) {
- if (command != CMD_NONE)
- fprintf(stderr, "Missing device number, DEVNO, for this command\n");
- usage(progname);
- return -1;
- }
+ if (device_selected) {
+ s = sdsi_create_dev(dev_no);
+ if (!s)
+ return -1;
- s = sdsi_create_dev(dev_no);
- if (!s)
- return -1;
+ switch (command) {
+ case CMD_SOCKET_INFO:
+ ret = sdsi_read_reg(s);
+ break;
+ case CMD_STATE_CERT:
+ ret = sdsi_state_cert_show(s);
+ break;
+ case CMD_PROV_AKC:
+ ret = sdsi_provision_akc(s, bin_file);
+ break;
+ case CMD_PROV_CAP:
+ ret = sdsi_provision_cap(s, bin_file);
+ break;
+ default:
+ fprintf(stderr, "No command specified\n");
+ return -1;
+ }
+
+ sdsi_free_dev(s);
- /* Run the command */
- switch (command) {
- case CMD_NONE:
- fprintf(stderr, "Missing command for device %s\n", dev_no);
- usage(progname);
- break;
- case CMD_SOCKET_INFO:
- ret = sdsi_read_reg(s);
- break;
- case CMD_DUMP_CERT:
- ret = sdsi_certificate_dump(s);
- break;
- case CMD_PROV_AKC:
- ret = sdsi_provision_akc(s, bin_file);
- break;
- case CMD_PROV_CAP:
- ret = sdsi_provision_cap(s, bin_file);
- break;
- }
-
-
- sdsi_free_dev(s);
+ } else {
+ fprintf(stderr, "No device specified\n");
+ return -1;
+ }
return ret;
}
--
2.25.1
On Tue, Nov 01, 2022 at 12:10:17PM -0700, David E. Box wrote:
> Newer versions of Intel On Demand hardware may have an expanded list of
> registers to support new features. The register layout is identified by a
> unique GUID that's read during driver probe. Add support for handling
> different GUIDs and add support for current GUIDs [1].
> [1] https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
Link: tag?
...
> #define SDSI_MIN_SIZE_DWORDS 276
> -#define SDSI_SIZE_CONTROL 8
> #define SDSI_SIZE_MAILBOX 1024
> -#define SDSI_SIZE_REGS 72
> +#define SDSI_SIZE_REGS 80
> #define SDSI_SIZE_CMD sizeof(u64)
> +#define SDSI_SIZE_MAILBOX 1024
Why do you need this second time?
...
> +static int sdsi_get_layout(struct sdsi_priv *priv, struct disc_table *table)
> +{
> + switch (table->guid) {
> + case SDSI_GUID_V1:
> + priv->control_size = 8;
> + priv->registers_size = 72;
> + break;
> + case SDSI_GUID_V2:
> + priv->control_size = 16;
> + priv->registers_size = 80;
Maybe it makes sense to use previously defined constants here instead of magics?
> + break;
> + default:
> + dev_err(priv->dev, "Unrecognized GUID 0x%x\n", table->guid);
> + return -EINVAL;
> + }
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
On Tue, Nov 01, 2022 at 12:10:18PM -0700, David E. Box wrote:
> Add support for reading the meter certificate from Intel On Demand
> hardware. The meter certificate [1] is used to access the utilization
> metrics of enabled features in support of the Intel On Demand consumption
> model. Similar to the state certificate, the meter certificate is read by
> mailbox command.
>
> [1] https://github.com/intel-sandbox/debox1.intel_sdsi/blob/gnr-review/meter-certificate.rst
Link: tag?
...
> static BIN_ATTR(state_certificate, 0400, state_certificate_read, NULL, SDSI_SIZE_READ_MSG);
BIN_ATTR_ADMiN_RO() ?
...
> +static BIN_ATTR(meter_certificate, 0400, meter_certificate_read, NULL, SDSI_SIZE_READ_MSG);
Ditto.
--
With Best Regards,
Andy Shevchenko
On Wed, 2022-11-02 at 12:44 +0200, Andy Shevchenko wrote:
> On Tue, Nov 01, 2022 at 12:10:17PM -0700, David E. Box wrote:
> > Newer versions of Intel On Demand hardware may have an expanded list of
> > registers to support new features. The register layout is identified by a
> > unique GUID that's read during driver probe. Add support for handling
> > different GUIDs and add support for current GUIDs [1].
> > [1] https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
>
> Link: tag?
Ack
>
> ...
>
> > #define SDSI_MIN_SIZE_DWORDS 276
> > -#define SDSI_SIZE_CONTROL 8
> > #define SDSI_SIZE_MAILBOX 1024
> > -#define SDSI_SIZE_REGS 72
> > +#define SDSI_SIZE_REGS 80
> > #define SDSI_SIZE_CMD sizeof(u64)
> > +#define SDSI_SIZE_MAILBOX 1024
>
> Why do you need this second time?
typo
>
> ...
>
> > +static int sdsi_get_layout(struct sdsi_priv *priv, struct disc_table
> > *table)
> > +{
> > + switch (table->guid) {
> > + case SDSI_GUID_V1:
> > + priv->control_size = 8;
> > + priv->registers_size = 72;
> > + break;
> > + case SDSI_GUID_V2:
> > + priv->control_size = 16;
> > + priv->registers_size = 80;
>
> Maybe it makes sense to use previously defined constants here instead of
> magics?
The constant is used for the file size, which since is static is set to the max.
But I'll add defines for these.
>
> > + break;
> > + default:
> > + dev_err(priv->dev, "Unrecognized GUID 0x%x\n", table->guid);
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
On Wed, 2022-11-02 at 12:46 +0200, Andy Shevchenko wrote:
> On Tue, Nov 01, 2022 at 12:10:18PM -0700, David E. Box wrote:
> > Add support for reading the meter certificate from Intel On Demand
> > hardware. The meter certificate [1] is used to access the utilization
> > metrics of enabled features in support of the Intel On Demand consumption
> > model. Similar to the state certificate, the meter certificate is read by
> > mailbox command.
> >
> > [1]
> > https://github.com/intel-sandbox/debox1.intel_sdsi/blob/gnr-review/meter-certificate.rst
>
> Link: tag?
>
> ...
>
> > static BIN_ATTR(state_certificate, 0400, state_certificate_read, NULL,
> > SDSI_SIZE_READ_MSG);
>
> BIN_ATTR_ADMiN_RO() ?
>
> ...
>
> > +static BIN_ATTR(meter_certificate, 0400, meter_certificate_read, NULL,
> > SDSI_SIZE_READ_MSG);
>
> Ditto.
>
Ack on all. Thanks Andy.
Hi David,
On 11/1/22 20:10, David E. Box wrote:
> Intel Software Defined Silicon (SDSi) is now known as Intel On Demand. The
> following patches do the following:
>
> 1. Identify the driver/tools as Intel On Demand. Only text descriptions are
> changed. Kconfig and filenames remain the same.
> 2. Perform some attribute cleanup by preventing the showing of files when
> features are not supported.
> 3. Adds support for a new GUID. GUIDs are used to identify the layout of
> the On Demand registers in sysfs. Layouts are described in the
> documentation on github [1].
> 4. Add support for reading On Demand meter certificates in sysfs.
> 5. The rest of the patches modify the existing tool to support discovery
> and reading of On Demand registers and the meter certificate.
>
> [1] https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
Sorry for the long silence, I have not done any pdx86 patch review
the last 2 weeks due to personal circumstances.
I will try to get this reviewed at the end of this week or the beginning
of next week.
Regards,
Hans
>
> David E. Box (9):
> platform/x86/intel/sdsi: Add Intel On Demand text
> platform/x86/intel/sdsi: Hide attributes if hardware doesn't support
> platform/x86/intel/sdsi: Support different GUIDs
> platform/x86/intel/sdsi: Add meter certificate support
> tools/arch/x86: intel_sdsi: Add support for reading state certificates
> tools/arch/x86: intel_sdsi: Add Intel On Demand text
> tools/arch/x86: intel_sdsi: Read more On Demand registers
> tools/arch/x86: intel_sdsi: Add support for new GUID
> tools/arch/x86: intel_sdsi: Add support for reading meter certificates
>
> .../ABI/testing/sysfs-driver-intel_sdsi | 47 +-
> drivers/platform/x86/intel/Kconfig | 8 +-
> drivers/platform/x86/intel/sdsi.c | 134 ++++-
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 458 ++++++++++++++----
> 4 files changed, 513 insertions(+), 134 deletions(-)
>
>
> base-commit: 225469d4acbcb873358d7618bad6e0203b67b964
Hi,
On 11/1/22 20:10, David E. Box wrote:
> Newer versions of Intel On Demand hardware may have an expanded list of
> registers to support new features. The register layout is identified by a
> unique GUID that's read during driver probe. Add support for handling
> different GUIDs and add support for current GUIDs [1].
>
> [1] https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
>
> Signed-off-by: David E. Box <[email protected]>
With Andy's remarks fixed this looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Regards,
Hans
> ---
> drivers/platform/x86/intel/sdsi.c | 47 +++++++++++++++++++++++++++++--
> 1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index bca05b4dd983..ab1f65919fc5 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -27,10 +27,10 @@
> #define ACCESS_TYPE_LOCAL 3
>
> #define SDSI_MIN_SIZE_DWORDS 276
> -#define SDSI_SIZE_CONTROL 8
> #define SDSI_SIZE_MAILBOX 1024
> -#define SDSI_SIZE_REGS 72
> +#define SDSI_SIZE_REGS 80
> #define SDSI_SIZE_CMD sizeof(u64)
> +#define SDSI_SIZE_MAILBOX 1024
>
> /*
> * Write messages are currently up to the size of the mailbox
> @@ -76,6 +76,9 @@
> #define DT_TBIR GENMASK(2, 0)
> #define DT_OFFSET(v) ((v) & GENMASK(31, 3))
>
> +#define SDSI_GUID_V1 0x006DD191
> +#define SDSI_GUID_V2 0xF210D9EF
> +
> enum sdsi_command {
> SDSI_CMD_PROVISION_AKC = 0x04,
> SDSI_CMD_PROVISION_CAP = 0x08,
> @@ -100,6 +103,9 @@ struct sdsi_priv {
> void __iomem *control_addr;
> void __iomem *mbox_addr;
> void __iomem *regs_addr;
> + int control_size;
> + int maibox_size;
> + int registers_size;
> u32 guid;
> u32 features;
> };
> @@ -444,6 +450,18 @@ static ssize_t registers_read(struct file *filp, struct kobject *kobj,
> struct device *dev = kobj_to_dev(kobj);
> struct sdsi_priv *priv = dev_get_drvdata(dev);
> void __iomem *addr = priv->regs_addr;
> + int size = priv->registers_size;
> +
> + /*
> + * The check below is performed by the sysfs caller based on the static
> + * file size. But this may be greater than the actual size which is based
> + * on the GUID. So check here again based on actual size before reading.
> + */
> + if (off >= size)
> + return 0;
> +
> + if (off + count > size)
> + count = size - off;
>
> memcpy_fromio(buf, addr + off, count);
>
> @@ -496,6 +514,24 @@ static const struct attribute_group sdsi_group = {
> };
> __ATTRIBUTE_GROUPS(sdsi);
>
> +static int sdsi_get_layout(struct sdsi_priv *priv, struct disc_table *table)
> +{
> + switch (table->guid) {
> + case SDSI_GUID_V1:
> + priv->control_size = 8;
> + priv->registers_size = 72;
> + break;
> + case SDSI_GUID_V2:
> + priv->control_size = 16;
> + priv->registers_size = 80;
> + break;
> + default:
> + dev_err(priv->dev, "Unrecognized GUID 0x%x\n", table->guid);
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> static int sdsi_map_mbox_registers(struct sdsi_priv *priv, struct pci_dev *parent,
> struct disc_table *disc_table, struct resource *disc_res)
> {
> @@ -537,7 +573,7 @@ static int sdsi_map_mbox_registers(struct sdsi_priv *priv, struct pci_dev *paren
> if (IS_ERR(priv->control_addr))
> return PTR_ERR(priv->control_addr);
>
> - priv->mbox_addr = priv->control_addr + SDSI_SIZE_CONTROL;
> + priv->mbox_addr = priv->control_addr + priv->control_size;
> priv->regs_addr = priv->mbox_addr + SDSI_SIZE_MAILBOX;
>
> priv->features = readq(priv->regs_addr + SDSI_ENABLED_FEATURES_OFFSET);
> @@ -572,6 +608,11 @@ static int sdsi_probe(struct auxiliary_device *auxdev, const struct auxiliary_de
>
> priv->guid = disc_table.guid;
>
> + /* Get guid based layout info */
> + ret = sdsi_get_layout(priv, &disc_table);
> + if (ret)
> + return ret;
> +
> /* Map the SDSi mailbox registers */
> ret = sdsi_map_mbox_registers(priv, intel_cap_dev->pcidev, &disc_table, disc_res);
> if (ret)
Hi,
On 11/1/22 20:10, David E. Box wrote:
> Add support for reading the meter certificate from Intel On Demand
> hardware. The meter certificate [1] is used to access the utilization
> metrics of enabled features in support of the Intel On Demand consumption
> model. Similar to the state certificate, the meter certificate is read by
> mailbox command.
>
> [1] https://github.com/intel-sandbox/debox1.intel_sdsi/blob/gnr-review/meter-certificate.rst
>
> Signed-off-by: David E. Box <[email protected]>
Besides Andy's remarks I also have 1 remark myself, see below.
> ---
> .../ABI/testing/sysfs-driver-intel_sdsi | 10 ++++
> drivers/platform/x86/intel/sdsi.c | 52 +++++++++++++++----
> 2 files changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel_sdsi b/Documentation/ABI/testing/sysfs-driver-intel_sdsi
> index 9d77f30d9b9a..f8afed127107 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel_sdsi
> +++ b/Documentation/ABI/testing/sysfs-driver-intel_sdsi
> @@ -69,6 +69,16 @@ Description:
> the CPU configuration is updated. A cold reboot is required to
> fully activate the feature. Mailbox command.
>
> +What: /sys/bus/auxiliary/devices/intel_vsec.sdsi.X/meter_certificate
> +Date: Nov 2022
> +KernelVersion: 6.2
> +Contact: "David E. Box" <[email protected]>
> +Description:
> + (RO) Used to read back the current meter certificate for the CPU
> + from Intel On Demand hardware. The meter certificate contains
> + utilization metrics of On Demand enabled features. Mailbox
> + command.
> +
> What: /sys/bus/auxiliary/devices/intel_vsec.sdsi.X/state_certificate
> Date: Feb 2022
> KernelVersion: 5.18
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index ab1f65919fc5..1dee10822df7 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -42,6 +42,7 @@
>
> #define SDSI_ENABLED_FEATURES_OFFSET 16
> #define SDSI_FEATURE_SDSI BIT(3)
> +#define SDSI_FEATURE_METERING BIT(26)
>
> #define SDSI_SOCKET_ID_OFFSET 64
> #define SDSI_SOCKET_ID GENMASK(3, 0)
> @@ -80,9 +81,10 @@
> #define SDSI_GUID_V2 0xF210D9EF
>
> enum sdsi_command {
> - SDSI_CMD_PROVISION_AKC = 0x04,
> - SDSI_CMD_PROVISION_CAP = 0x08,
> - SDSI_CMD_READ_STATE = 0x10,
> + SDSI_CMD_PROVISION_AKC = 0x0004,
> + SDSI_CMD_PROVISION_CAP = 0x0008,
> + SDSI_CMD_READ_STATE = 0x0010,
> + SDSI_CMD_READ_METER = 0x0014,
> };
>
> struct sdsi_mbox_info {
> @@ -398,13 +400,10 @@ static ssize_t provision_cap_write(struct file *filp, struct kobject *kobj,
> }
> static BIN_ATTR_WO(provision_cap, SDSI_SIZE_WRITE_MSG);
>
> -static long state_certificate_read(struct file *filp, struct kobject *kobj,
> - struct bin_attribute *attr, char *buf, loff_t off,
> - size_t count)
> +static ssize_t
> +certificate_read(u64 command, struct sdsi_priv *priv, char *buf, loff_t off,
> + size_t count)
> {
> - struct device *dev = kobj_to_dev(kobj);
> - struct sdsi_priv *priv = dev_get_drvdata(dev);
> - u64 command = SDSI_CMD_READ_STATE;
> struct sdsi_mbox_info info;
> size_t size;
> int ret;
> @@ -441,8 +440,31 @@ static long state_certificate_read(struct file *filp, struct kobject *kobj,
>
> return size;
> }
> +
> +static ssize_t
> +state_certificate_read(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *attr, char *buf, loff_t off,
> + size_t count)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct sdsi_priv *priv = dev_get_drvdata(dev);
> +
> + return certificate_read(SDSI_CMD_READ_STATE, priv, buf, off, count);
> +}
> static BIN_ATTR(state_certificate, 0400, state_certificate_read, NULL, SDSI_SIZE_READ_MSG);
>
> +static ssize_t
> +meter_certificate_read(struct file *filp, struct kobject *kobj,
> + struct bin_attribute *attr, char *buf, loff_t off,
> + size_t count)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct sdsi_priv *priv = dev_get_drvdata(dev);
> +
> + return certificate_read(SDSI_CMD_READ_METER, priv, buf, off, count);
> +}
> +static BIN_ATTR(meter_certificate, 0400, meter_certificate_read, NULL, SDSI_SIZE_READ_MSG);
> +
> static ssize_t registers_read(struct file *filp, struct kobject *kobj,
> struct bin_attribute *attr, char *buf, loff_t off,
> size_t count)
> @@ -472,6 +494,7 @@ static BIN_ATTR(registers, 0400, registers_read, NULL, SDSI_SIZE_REGS);
> static struct bin_attribute *sdsi_bin_attrs[] = {
> &bin_attr_registers,
> &bin_attr_state_certificate,
> + &bin_attr_meter_certificate,
> &bin_attr_provision_akc,
> &bin_attr_provision_cap,
> NULL
> @@ -491,7 +514,16 @@ sdsi_battr_is_visible(struct kobject *kobj, struct bin_attribute *attr, int n)
> if (!(priv->features & SDSI_FEATURE_SDSI))
> return 0;
>
> - return attr->attr.mode;
> + if (attr == &bin_attr_state_certificate ||
> + attr == &bin_attr_provision_akc ||
> + attr == &bin_attr_provision_cap)
> + return attr->attr.mode;
I would prefer for you to just drop this and then change:
> +
> + if (attr == &bin_attr_meter_certificate &&
> + !!(priv->features & SDSI_FEATURE_METERING))
> + return attr->attr.mode;
to:
if (attr == &bin_attr_meter_certificate)
return (priv->features & SDSI_FEATURE_METERING) ?
attr->attr.mode : 0;
And then keep:
return attr->attr.mode;
at the end of this function, because now you are hiding all
new attributes by default and then you have to add any new
attributes to the if above, leading to an ever growing lists
of attr to check in that if, so just having:
return attr->attr.mode;
As default for any non-matches attr would be much better IMHO.
Regards,
Hans
> +
> + return 0;
> }
>
> static ssize_t guid_show(struct device *dev, struct device_attribute *attr, char *buf)
Hi,
On 11/1/22 20:10, David E. Box wrote:
> Intel Software Defined Silicon (SDSi) is now officially known as Intel
> On Demand. Add On Demand to the description in the kconfig, documentation,
> and driver source.
>
> Signed-off-by: David E. Box <[email protected]>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Regards,
Hans
> ---
> .../ABI/testing/sysfs-driver-intel_sdsi | 37 ++++++++++---------
> drivers/platform/x86/intel/Kconfig | 8 ++--
> drivers/platform/x86/intel/sdsi.c | 4 +-
> 3 files changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel_sdsi b/Documentation/ABI/testing/sysfs-driver-intel_sdsi
> index 96b92c105ec4..9d77f30d9b9a 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel_sdsi
> +++ b/Documentation/ABI/testing/sysfs-driver-intel_sdsi
> @@ -4,21 +4,21 @@ KernelVersion: 5.18
> Contact: "David E. Box" <[email protected]>
> Description:
> This directory contains interface files for accessing Intel
> - Software Defined Silicon (SDSi) features on a CPU. X
> - represents the socket instance (though not the socket ID).
> - The socket ID is determined by reading the registers file
> - and decoding it per the specification.
> + On Demand (formerly Software Defined Silicon or SDSi) features
> + on a CPU. X represents the socket instance (though not the
> + socket ID). The socket ID is determined by reading the
> + registers file and decoding it per the specification.
>
> - Some files communicate with SDSi hardware through a mailbox.
> - Should the operation fail, one of the following error codes
> - may be returned:
> + Some files communicate with On Demand hardware through a
> + mailbox. Should the operation fail, one of the following error
> + codes may be returned:
>
> ========== =====
> Error Code Cause
> ========== =====
> EIO General mailbox failure. Log may indicate cause.
> EBUSY Mailbox is owned by another agent.
> - EPERM SDSI capability is not enabled in hardware.
> + EPERM On Demand capability is not enabled in hardware.
> EPROTO Failure in mailbox protocol detected by driver.
> See log for details.
> EOVERFLOW For provision commands, the size of the data
> @@ -54,8 +54,8 @@ KernelVersion: 5.18
> Contact: "David E. Box" <[email protected]>
> Description:
> (WO) Used to write an Authentication Key Certificate (AKC) to
> - the SDSi NVRAM for the CPU. The AKC is used to authenticate a
> - Capability Activation Payload. Mailbox command.
> + the On Demand NVRAM for the CPU. The AKC is used to authenticate
> + a Capability Activation Payload. Mailbox command.
>
> What: /sys/bus/auxiliary/devices/intel_vsec.sdsi.X/provision_cap
> Date: Feb 2022
> @@ -63,17 +63,18 @@ KernelVersion: 5.18
> Contact: "David E. Box" <[email protected]>
> Description:
> (WO) Used to write a Capability Activation Payload (CAP) to the
> - SDSi NVRAM for the CPU. CAPs are used to activate a given CPU
> - feature. A CAP is validated by SDSi hardware using a previously
> - provisioned AKC file. Upon successful authentication, the CPU
> - configuration is updated. A cold reboot is required to fully
> - activate the feature. Mailbox command.
> + On Demand NVRAM for the CPU. CAPs are used to activate a given
> + CPU feature. A CAP is validated by On Demand hardware using a
> + previously provisioned AKC file. Upon successful authentication,
> + the CPU configuration is updated. A cold reboot is required to
> + fully activate the feature. Mailbox command.
>
> What: /sys/bus/auxiliary/devices/intel_vsec.sdsi.X/state_certificate
> Date: Feb 2022
> KernelVersion: 5.18
> Contact: "David E. Box" <[email protected]>
> Description:
> - (RO) Used to read back the current State Certificate for the CPU
> - from SDSi hardware. The State Certificate contains information
> - about the current licenses on the CPU. Mailbox command.
> + (RO) Used to read back the current state certificate for the CPU
> + from On Demand hardware. The state certificate contains
> + information about the current licenses on the CPU. Mailbox
> + command.
> diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig
> index 794968bda115..d5a33473e838 100644
> --- a/drivers/platform/x86/intel/Kconfig
> +++ b/drivers/platform/x86/intel/Kconfig
> @@ -157,13 +157,13 @@ config INTEL_RST
> as usual.
>
> config INTEL_SDSI
> - tristate "Intel Software Defined Silicon Driver"
> + tristate "Intel On Demand (Software Defined Silicon) Driver"
> depends on INTEL_VSEC
> depends on X86_64
> help
> - This driver enables access to the Intel Software Defined Silicon
> - interface used to provision silicon features with an authentication
> - certificate and capability license.
> + This driver enables access to the Intel On Demand (formerly Software
> + Defined Silicon) interface used to provision silicon features with an
> + authentication certificate and capability license.
>
> To compile this driver as a module, choose M here: the module will
> be called intel_sdsi.
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index c830e98dfa38..32793919473d 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Intel Software Defined Silicon driver
> + * Intel On Demand (Software Defined Silicon) driver
> *
> * Copyright (c) 2022, Intel Corporation.
> * All Rights Reserved.
> @@ -586,5 +586,5 @@ static struct auxiliary_driver sdsi_aux_driver = {
> module_auxiliary_driver(sdsi_aux_driver);
>
> MODULE_AUTHOR("David E. Box <[email protected]>");
> -MODULE_DESCRIPTION("Intel Software Defined Silicon driver");
> +MODULE_DESCRIPTION("Intel On Demand (SDSi) driver");
> MODULE_LICENSE("GPL");
Hi,
On 11/1/22 20:10, David E. Box wrote:
> The structure and content of the On Demand registers is based on the GUID
> which is read from hardware through sysfs. Add support for decoding the
> registers of a new GUID 0xF210D9EF.
>
> Signed-off-by: David E. Box <[email protected]>
> ---
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 32 ++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index 01b5f9994e11..0680eda78b1a 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -35,7 +35,8 @@
> #define SDSI_DEV "intel_vsec.sdsi"
> #define AUX_DEV_PATH "/sys/bus/auxiliary/devices/"
> #define SDSI_PATH (AUX_DEV_DIR SDSI_DEV)
> -#define GUID 0x6dd191
> +#define GUID_V1 0x6dd191
> +#define GUID_V2 0xF210D9EF
> #define REGISTERS_MIN_SIZE 72
> #define STATE_CERT_MAX_SIZE 4096
> #define STATE_MAX_NUM_LICENSES 16
> @@ -100,9 +101,17 @@ struct sdsi_regs {
> struct availability prov_avail;
> struct nvram_update_limit limits;
> uint64_t pcu_cr3_capid_cfg;
> - uint64_t socket_id;
> + union {
> + struct {
> + uint64_t socket_id;
> + } v1;
> + struct {
> + uint64_t reserved;
> + uint64_t socket_id;
> + uint64_t reserved2;
> + } v2;
> + } extra;
> };
> -
> #define CONTENT_TYPE_LK_ENC 0xD
> #define CONTENT_TYPE_LK_BLOB_ENC 0xE
>
> @@ -146,7 +155,7 @@ struct sdsi_dev {
> struct state_certificate sc;
> char *dev_name;
> char *dev_path;
> - int guid;
> + uint32_t guid;
> };
>
> enum command {
> @@ -199,7 +208,7 @@ static int sdsi_update_registers(struct sdsi_dev *s)
> return -1;
> }
>
> - if (s->guid != GUID) {
> + if (s->guid != GUID_V1 && s->guid != GUID_V2) {
> fprintf(stderr, "Unrecognized guid, 0x%x\n", s->guid);
> fclose(regs_ptr);
> return -1;
> @@ -207,7 +216,7 @@ static int sdsi_update_registers(struct sdsi_dev *s)
>
> /* Update register info for this guid */
> ret = fread(&s->regs, sizeof(uint8_t), sizeof(s->regs), regs_ptr);
> - if (ret != sizeof(s->regs)) {
> + if (ret > (int)sizeof(s->regs)) { /* FIXME: Check size by guid */
This is wrong, fread will never return more then requested, that
would lead to buffer overflows. But it may return 0 on errors, or
a short read on an error.
So you need to fix the FIXME comment you added here as now you
have just disabled all error checking.
Regards,
Hans
> fprintf(stderr, "Could not read 'registers' file\n");
> fclose(regs_ptr);
> return -1;
> @@ -252,10 +261,13 @@ static int sdsi_read_reg(struct sdsi_dev *s)
> printf(" Updates Available: %d\n", s->regs.prov_avail.available);
> printf(" Updates Threshold: %d\n", s->regs.prov_avail.threshold);
> printf("NVRAM Udate Limit\n");
> - printf(" 50%% Limit Reached: %s\n", !!s->regs.limits.sdsi_50_pct ? "Yes" : "No");
> - printf(" 75%% Limit Reached: %s\n", !!s->regs.limits.sdsi_75_pct ? "Yes" : "No");
> - printf(" 90%% Limit Reached: %s\n", !!s->regs.limits.sdsi_90_pct ? "Yes" : "No");
> - printf("Socket ID: %ld\n", s->regs.socket_id & 0xF);
> + printf(" 50%% Limit Reached: %s\n", !!s->regs.limits.sdsi_50_pct ? "Yes" : "No");
> + printf(" 75%% Limit Reached: %s\n", !!s->regs.limits.sdsi_75_pct ? "Yes" : "No");
> + printf(" 90%% Limit Reached: %s\n", !!s->regs.limits.sdsi_90_pct ? "Yes" : "No");
> + if (s->guid == GUID_V1)
> + printf("Socket ID: %ld\n", s->regs.extra.v1.socket_id & 0xF);
> + else
> + printf("Socket ID: %ld\n", s->regs.extra.v2.socket_id & 0xF);
>
> return 0;
> }
Hi David,
On 11/1/22 20:10, David E. Box wrote:
> Intel Software Defined Silicon (SDSi) is now known as Intel On Demand. The
> following patches do the following:
>
> 1. Identify the driver/tools as Intel On Demand. Only text descriptions are
> changed. Kconfig and filenames remain the same.
> 2. Perform some attribute cleanup by preventing the showing of files when
> features are not supported.
> 3. Adds support for a new GUID. GUIDs are used to identify the layout of
> the On Demand registers in sysfs. Layouts are described in the
> documentation on github [1].
> 4. Add support for reading On Demand meter certificates in sysfs.
> 5. The rest of the patches modify the existing tool to support discovery
> and reading of On Demand registers and the meter certificate.
>
> [1] https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
>
> David E. Box (9):
> platform/x86/intel/sdsi: Add Intel On Demand text
> platform/x86/intel/sdsi: Hide attributes if hardware doesn't support
> platform/x86/intel/sdsi: Support different GUIDs
> platform/x86/intel/sdsi: Add meter certificate support
> tools/arch/x86: intel_sdsi: Add support for reading state certificates
> tools/arch/x86: intel_sdsi: Add Intel On Demand text
> tools/arch/x86: intel_sdsi: Read more On Demand registers
> tools/arch/x86: intel_sdsi: Add support for new GUID
> tools/arch/x86: intel_sdsi: Add support for reading meter certificates
Thank you, over all this looks good. I have some small remarks
on patches 4, 8 and 9 see my replies to those.
Please prepare a v2 addressing Andy's + my review remarks and get
that v2 to me no later then next week Tuesday, then I can still
merge this in time for 6.2 .
Regards,
Hans
Hi,
On 11/1/22 20:10, David E. Box wrote:
> Intel Software Defined Silicon (SDSi) is now officially known as Intel
> On Demand. Change text in tool to indicate this.
>
> Signed-off-by: David E. Box <[email protected]>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Regards,
Hans
> ---
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index 9dd94014a672..3718bd0c05cb 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * sdsi: Intel Software Defined Silicon tool for provisioning certificates
> - * and activation payloads on supported cpus.
> + * sdsi: Intel On Demand (formerly Software Defined Silicon) tool for
> + * provisioning certificates and activation payloads on supported cpus.
> *
> * See https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
> * for register descriptions.
> @@ -150,7 +150,7 @@ static void sdsi_list_devices(void)
> }
>
> if (!found)
> - fprintf(stderr, "No sdsi devices found.\n");
> + fprintf(stderr, "No On Demand devices found.\n");
> }
>
> static int sdsi_update_registers(struct sdsi_dev *s)
> @@ -206,7 +206,7 @@ static int sdsi_read_reg(struct sdsi_dev *s)
> printf("\n");
> printf("PPIN: 0x%lx\n", s->regs.ppin);
> printf("Enabled Features\n");
> - printf(" SDSi: %s\n", !!s->regs.en_features.sdsi ? "Enabled" : "Disabled");
> + printf(" On Demand: %s\n", !!s->regs.en_features.sdsi ? "Enabled" : "Disabled");
> printf("Authorization Failure Count\n");
> printf(" AKC Failure Count: %d\n", s->regs.auth_fail_count.key_failure_count);
> printf(" AKC Failure Threshold: %d\n", s->regs.auth_fail_count.key_failure_threshold);
> @@ -428,7 +428,7 @@ static int sdsi_provision_akc(struct sdsi_dev *s, char *bin_file)
> return ret;
>
> if (!s->regs.en_features.sdsi) {
> - fprintf(stderr, "SDSi feature is present but not enabled. Unable to provision");
> + fprintf(stderr, "On Demand feature is present but not enabled. Unable to provision");
> return -1;
> }
>
> @@ -458,7 +458,7 @@ static int sdsi_provision_cap(struct sdsi_dev *s, char *bin_file)
> return ret;
>
> if (!s->regs.en_features.sdsi) {
> - fprintf(stderr, "SDSi feature is present but not enabled. Unable to provision");
> + fprintf(stderr, "On Demand feature is present but not enabled. Unable to provision");
> return -1;
> }
>
Hi,
On 11/1/22 20:10, David E. Box wrote:
> Add option to read and decode On Demand state certificates.
>
> Link: https://github.com/intel/intel-sdsi/blob/master/state-certificate-encoding.rst
>
> Signed-off-by: David E. Box <[email protected]>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Regards,
Hans
> ---
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 268 ++++++++++++++++++-------
> 1 file changed, 198 insertions(+), 70 deletions(-)
>
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index c0e2f2349db4..9dd94014a672 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -22,11 +22,24 @@
>
> #include <sys/types.h>
>
> +#ifndef __packed
> +#define __packed __attribute__((packed))
> +#endif
> +
> +#define min(x, y) ({ \
> + typeof(x) _min1 = (x); \
> + typeof(y) _min2 = (y); \
> + (void) (&_min1 == &_min2); \
> + _min1 < _min2 ? _min1 : _min2; })
> +
> #define SDSI_DEV "intel_vsec.sdsi"
> #define AUX_DEV_PATH "/sys/bus/auxiliary/devices/"
> #define SDSI_PATH (AUX_DEV_DIR SDSI_DEV)
> #define GUID 0x6dd191
> #define REGISTERS_MIN_SIZE 72
> +#define STATE_CERT_MAX_SIZE 4096
> +#define STATE_MAX_NUM_LICENSES 16
> +#define STATE_MAX_NUM_IN_BUNDLE (uint32_t)8
>
> #define __round_mask(x, y) ((__typeof__(x))((y) - 1))
> #define round_up(x, y) ((((x) - 1) | __round_mask(x, y)) + 1)
> @@ -49,6 +62,7 @@ struct availability {
> uint64_t reserved:48;
> uint64_t available:3;
> uint64_t threshold:3;
> + uint64_t reserved2:10;
> };
>
> struct sdsi_regs {
> @@ -63,17 +77,55 @@ struct sdsi_regs {
> uint64_t socket_id;
> };
>
> +#define CONTENT_TYPE_LK_ENC 0xD
> +#define CONTENT_TYPE_LK_BLOB_ENC 0xE
> +
> +struct state_certificate {
> + uint32_t content_type;
> + uint32_t region_rev_id;
> + uint32_t header_size;
> + uint32_t total_size;
> + uint32_t key_size;
> + uint32_t num_licenses;
> +};
> +
> +struct license_key_info {
> + uint32_t key_rev_id;
> + uint64_t key_image_content[6];
> +} __packed;
> +
> +#define LICENSE_BLOB_SIZE(l) (((l) & 0x7fffffff) * 4)
> +#define LICENSE_VALID(l) (!!((l) & 0x80000000))
> +
> +// License Group Types
> +#define LBT_ONE_TIME_UPGRADE 1
> +#define LBT_METERED_UPGRADE 2
> +
> +struct license_blob_content {
> + uint32_t type;
> + uint64_t id;
> + uint64_t ppin;
> + uint64_t previous_ppin;
> + uint32_t rev_id;
> + uint32_t num_bundles;
> +} __packed;
> +
> +struct bundle_encoding {
> + uint32_t encoding;
> + uint32_t encoding_rsvd[7];
> +};
> +
> struct sdsi_dev {
> struct sdsi_regs regs;
> + struct state_certificate sc;
> char *dev_name;
> char *dev_path;
> int guid;
> };
>
> enum command {
> - CMD_NONE,
> CMD_SOCKET_INFO,
> - CMD_DUMP_CERT,
> + CMD_STATE_CERT,
> CMD_PROV_AKC,
> CMD_PROV_CAP,
> };
> @@ -168,20 +220,56 @@ static int sdsi_read_reg(struct sdsi_dev *s)
> return 0;
> }
>
> -static int sdsi_certificate_dump(struct sdsi_dev *s)
> +static char *license_blob_type(uint32_t type)
> +{
> + switch (type) {
> + case LBT_ONE_TIME_UPGRADE:
> + return "One time upgrade";
> + case LBT_METERED_UPGRADE:
> + return "Metered upgrade";
> + default:
> + return "Unknown license blob type";
> + }
> +}
> +
> +static char *content_type(uint32_t type)
> +{
> + switch (type) {
> + case CONTENT_TYPE_LK_ENC:
> + return "Licencse key encoding";
> + case CONTENT_TYPE_LK_BLOB_ENC:
> + return "License key + Blob encoding";
> + default:
> + return "Unknown content type";
> + }
> +}
> +
> +static void get_feature(uint32_t encoding, char *feature)
> +{
> + char *name = (char *)&encoding;
> +
> + feature[3] = name[0];
> + feature[2] = name[1];
> + feature[1] = name[2];
> + feature[0] = name[3];
> +}
> +
> +static int sdsi_state_cert_show(struct sdsi_dev *s)
> {
> - uint64_t state_certificate[512] = {0};
> - bool first_instance;
> - uint64_t previous;
> + char buf[STATE_CERT_MAX_SIZE] = {0};
> + struct state_certificate *sc;
> + struct license_key_info *lki;
> + uint32_t offset = 0;
> + uint32_t count = 0;
> FILE *cert_ptr;
> - int i, ret, size;
> + 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.");
> + fprintf(stderr, "On Demand feature is present but not enabled.");
> fprintf(stderr, " Unable to read state certificate");
> return -1;
> }
> @@ -198,32 +286,74 @@ static int sdsi_certificate_dump(struct sdsi_dev *s)
> return -1;
> }
>
> - size = fread(state_certificate, 1, sizeof(state_certificate), cert_ptr);
> + size = fread(buf, 1, sizeof(buf), cert_ptr);
> if (!size) {
> fprintf(stderr, "Could not read 'state_certificate' file\n");
> fclose(cert_ptr);
> return -1;
> }
> + fclose(cert_ptr);
>
> - printf("%3d: 0x%lx\n", 0, state_certificate[0]);
> - previous = state_certificate[0];
> - first_instance = true;
> + sc = (struct state_certificate *)buf;
>
> - for (i = 1; i < (int)(round_up(size, sizeof(uint64_t))/sizeof(uint64_t)); i++) {
> - if (state_certificate[i] == previous) {
> - if (first_instance) {
> - puts("*");
> - first_instance = false;
> - }
> - continue;
> + /* Print register info for this guid */
> + printf("\n");
> + printf("State certificate for device %s\n", s->dev_name);
> + printf("\n");
> + printf("Content Type: %s\n", content_type(sc->content_type));
> + printf("Region Revision ID: %d\n", sc->region_rev_id);
> + printf("Header Size: %d\n", sc->header_size * 4);
> + printf("Total Size: %d\n", sc->total_size);
> + printf("OEM Key Size: %d\n", sc->key_size * 4);
> + printf("Number of Licenses: %d\n", sc->num_licenses);
> +
> + /* Skip over the license sizes 4 bytes per license) to get the license key info */
> + lki = (void *)sc + sizeof(*sc) + (4 * sc->num_licenses);
> +
> + printf("License blob Info:\n");
> + printf(" License Key Revision ID: 0x%x\n", lki->key_rev_id);
> + printf(" License Key Image Content: 0x%lx%lx%lx%lx%lx%lx\n",
> + lki->key_image_content[5], lki->key_image_content[4],
> + lki->key_image_content[3], lki->key_image_content[2],
> + lki->key_image_content[1], lki->key_image_content[0]);
> +
> + while (count++ < sc->num_licenses) {
> + uint32_t blob_size_field = *(uint32_t *)(buf + 0x14 + count * 4);
> + uint32_t blob_size = LICENSE_BLOB_SIZE(blob_size_field);
> + bool license_valid = LICENSE_VALID(blob_size_field);
> + struct license_blob_content *lbc =
> + (void *)(sc) + // start of the state certificate
> + sizeof(*sc) + // size of the state certificate
> + (4 * sc->num_licenses) + // total size of the blob size blocks
> + sizeof(*lki) + // size of the license key info
> + offset; // offset to this blob content
> + struct bundle_encoding *bundle = (void *)(lbc) + sizeof(*lbc);
> + char feature[5];
> + uint32_t i;
> +
> + printf(" Blob %d:\n", count - 1);
> + printf(" License blob size: %u\n", blob_size);
> + printf(" License is valid: %s\n", license_valid ? "Yes" : "No");
> + printf(" License blob type: %s\n", license_blob_type(lbc->type));
> + printf(" License blob ID: 0x%lx\n", lbc->id);
> + printf(" PPIN: 0x%lx\n", lbc->ppin);
> + printf(" Previous PPIN: 0x%lx\n", lbc->previous_ppin);
> + printf(" Blob revision ID: %u\n", lbc->rev_id);
> + printf(" Number of Features: %u\n", lbc->num_bundles);
> +
> + feature[4] = '\0';
> +
> + for (i = 0; i < min(lbc->num_bundles, STATE_MAX_NUM_IN_BUNDLE); i++) {
> + get_feature(bundle[i].encoding, feature);
> + printf(" Feature %d: %s\n", i, feature);
> }
> - printf("%3d: 0x%lx\n", i, state_certificate[i]);
> - previous = state_certificate[i];
> - first_instance = true;
> - }
> - printf("%3d\n", i);
>
> - fclose(cert_ptr);
> + if (lbc->num_bundles > STATE_MAX_NUM_IN_BUNDLE)
> + fprintf(stderr, " Warning: %d > %d licenses in bundle reported.\n",
> + lbc->num_bundles, STATE_MAX_NUM_IN_BUNDLE);
> +
> + offset += blob_size;
> + };
>
> return 0;
> }
> @@ -231,7 +361,7 @@ static int sdsi_certificate_dump(struct sdsi_dev *s)
> static int sdsi_provision(struct sdsi_dev *s, char *bin_file, enum command command)
> {
> int bin_fd, prov_fd, size, ret;
> - char buf[4096] = { 0 };
> + char buf[STATE_CERT_MAX_SIZE] = { 0 };
> char cap[] = "provision_cap";
> char akc[] = "provision_akc";
> char *prov_file;
> @@ -266,7 +396,7 @@ static int sdsi_provision(struct sdsi_dev *s, char *bin_file, enum command comma
> }
>
> /* Read the binary file into the buffer */
> - size = read(bin_fd, buf, 4096);
> + size = read(bin_fd, buf, STATE_CERT_MAX_SIZE);
> if (size == -1) {
> close(bin_fd);
> close(prov_fd);
> @@ -443,25 +573,26 @@ static void sdsi_free_dev(struct sdsi_dev *s)
>
> static void usage(char *prog)
> {
> - printf("Usage: %s [-l] [-d DEVNO [-iD] [-a FILE] [-c FILE]]\n", prog);
> + printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-a FILE] [-c FILE]]\n", prog);
> }
>
> static void show_help(void)
> {
> printf("Commands:\n");
> - printf(" %-18s\t%s\n", "-l, --list", "list available sdsi devices");
> - printf(" %-18s\t%s\n", "-d, --devno DEVNO", "sdsi device number");
> - printf(" %-18s\t%s\n", "-i --info", "show socket information");
> - printf(" %-18s\t%s\n", "-D --dump", "dump state certificate 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");
> + 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", "-a, --akc FILE", "provision socket with AKC FILE");
> + printf(" %-18s\t%s\n", "-c, --cap FILE>", "provision socket with CAP FILE");
> }
>
> int main(int argc, char *argv[])
> {
> char bin_file[PATH_MAX], *dev_no = NULL;
> + bool device_selected = false;
> char *progname;
> - enum command command = CMD_NONE;
> + enum command command = -1;
> struct sdsi_dev *s;
> int ret = 0, opt;
> int option_index = 0;
> @@ -470,21 +601,22 @@ int main(int argc, char *argv[])
> {"akc", required_argument, 0, 'a'},
> {"cap", required_argument, 0, 'c'},
> {"devno", required_argument, 0, 'd'},
> - {"dump", no_argument, 0, 'D'},
> {"help", no_argument, 0, 'h'},
> {"info", no_argument, 0, 'i'},
> {"list", no_argument, 0, 'l'},
> + {"state", no_argument, 0, 's'},
> {0, 0, 0, 0 }
> };
>
>
> progname = argv[0];
>
> - while ((opt = getopt_long_only(argc, argv, "+a:c:d:Da:c:h", long_options,
> + while ((opt = getopt_long_only(argc, argv, "+a:c:d:hils", long_options,
> &option_index)) != -1) {
> switch (opt) {
> case 'd':
> dev_no = optarg;
> + device_selected = true;
> break;
> case 'l':
> sdsi_list_devices();
> @@ -492,8 +624,8 @@ int main(int argc, char *argv[])
> case 'i':
> command = CMD_SOCKET_INFO;
> break;
> - case 'D':
> - command = CMD_DUMP_CERT;
> + case 's':
> + command = CMD_STATE_CERT;
> break;
> case 'a':
> case 'c':
> @@ -520,39 +652,35 @@ int main(int argc, char *argv[])
> }
> }
>
> - if (!dev_no) {
> - if (command != CMD_NONE)
> - fprintf(stderr, "Missing device number, DEVNO, for this command\n");
> - usage(progname);
> - return -1;
> - }
> + if (device_selected) {
> + s = sdsi_create_dev(dev_no);
> + if (!s)
> + return -1;
>
> - s = sdsi_create_dev(dev_no);
> - if (!s)
> - return -1;
> + switch (command) {
> + case CMD_SOCKET_INFO:
> + ret = sdsi_read_reg(s);
> + break;
> + case CMD_STATE_CERT:
> + ret = sdsi_state_cert_show(s);
> + break;
> + case CMD_PROV_AKC:
> + ret = sdsi_provision_akc(s, bin_file);
> + break;
> + case CMD_PROV_CAP:
> + ret = sdsi_provision_cap(s, bin_file);
> + break;
> + default:
> + fprintf(stderr, "No command specified\n");
> + return -1;
> + }
> +
> + sdsi_free_dev(s);
>
> - /* Run the command */
> - switch (command) {
> - case CMD_NONE:
> - fprintf(stderr, "Missing command for device %s\n", dev_no);
> - usage(progname);
> - break;
> - case CMD_SOCKET_INFO:
> - ret = sdsi_read_reg(s);
> - break;
> - case CMD_DUMP_CERT:
> - ret = sdsi_certificate_dump(s);
> - break;
> - case CMD_PROV_AKC:
> - ret = sdsi_provision_akc(s, bin_file);
> - break;
> - case CMD_PROV_CAP:
> - ret = sdsi_provision_cap(s, bin_file);
> - break;
> - }
> -
> -
> - sdsi_free_dev(s);
> + } else {
> + fprintf(stderr, "No device specified\n");
> + return -1;
> + }
>
> return ret;
> }
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 <[email protected]>
> ---
> 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 <[email protected]>
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;
On Thu, 2022-11-17 at 15:01 +0100, Hans de Goede wrote:
> Hi David,
>
> On 11/1/22 20:10, David E. Box wrote:
> > Intel Software Defined Silicon (SDSi) is now known as Intel On Demand. The
> > following patches do the following:
> >
> > 1. Identify the driver/tools as Intel On Demand. Only text descriptions are
> > changed. Kconfig and filenames remain the same.
> > 2. Perform some attribute cleanup by preventing the showing of files when
> > features are not supported.
> > 3. Adds support for a new GUID. GUIDs are used to identify the layout of
> > the On Demand registers in sysfs. Layouts are described in the
> > documentation on github [1].
> > 4. Add support for reading On Demand meter certificates in sysfs.
> > 5. The rest of the patches modify the existing tool to support discovery
> > and reading of On Demand registers and the meter certificate.
> >
> > [1] https://github.com/intel/intel-sdsi/blob/master/os-interface.rst
> >
> > David E. Box (9):
> > platform/x86/intel/sdsi: Add Intel On Demand text
> > platform/x86/intel/sdsi: Hide attributes if hardware doesn't support
> > platform/x86/intel/sdsi: Support different GUIDs
> > platform/x86/intel/sdsi: Add meter certificate support
> > tools/arch/x86: intel_sdsi: Add support for reading state certificates
> > tools/arch/x86: intel_sdsi: Add Intel On Demand text
> > tools/arch/x86: intel_sdsi: Read more On Demand registers
> > tools/arch/x86: intel_sdsi: Add support for new GUID
> > tools/arch/x86: intel_sdsi: Add support for reading meter certificates
>
> Thank you, over all this looks good. I have some small remarks
> on patches 4, 8 and 9 see my replies to those.
>
> Please prepare a v2 addressing Andy's + my review remarks and get
> that v2 to me no later then next week Tuesday, then I can still
> merge this in time for 6.2 .
Will do. Thanks Hans, Andy.
>
> Regards,
>
> Hans
>
>
Hi!
> Intel Software Defined Silicon (SDSi) is now known as Intel On Demand. The
> following patches do the following:
Quick google gets me to
https://www.intel.com/content/www/us/en/products/docs/ondemand/overview.html
... which is not really clear.
Do I understand it correctly that this is support for unlocking
features on silicon user already owns?
That is rather user hostile, is it?
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.