* [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation
@ 2024-02-01 1:07 David E. Box
2024-02-01 1:07 ` [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes David E. Box
` (9 more replies)
0 siblings, 10 replies; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
This patch series primarily adds support for a new netlink ABI in the
Intel On Demand driver for performing attestation of the hardware state.
Attestation patches
Patch 1: The attestation mailbox command requires that the message size
register be set along with the package size. Adds that support.
Patch 2: The attestation command will need to write the SPDM message and
read the response. The current mailbox flow handles reads and writes
separately. Combines the two flows.
Patch 3: Patch 4 will create a separate c file for the netlink
interface. Add a separate header file now. No functional changes. This
mostly just makes it easier to see the changes in Patch 4.
Patch 4: Adds attestation support to the driver and provides a netlink
interface to perform the service.
Other changes
Patch 5: Adds support to read the in-band BIOS lock. If set, On Demand
controls are not available in the driver.
Patch 6: Adds a new attribute to allow reading the most current metering
state.
Patch 7: Fixes for the intel_sdsi tool
Patch 8: Adds support to the intel_sdsi tool to read the current meter
state.
David E. Box (7):
platform/x86/intel/sdsi: Set message size during writes
platform/x86/intel/sdsi: Combine read and write mailbox flows
platform/x86/intel/sdsi: Add header file
platform/x86/intel/sdsi: Add netlink SPDM transport
platform/x86/intel/sdsi: Add attribute to read the current meter state
tools: Fix errors in meter_certificate display
tools: intel_sdsi: Add current meter support
Kuppuswamy Sathyanarayanan (1):
platform/x86/intel/sdsi: Add in-band BIOS lock support
Documentation/netlink/specs/intel_sdsi.yaml | 97 ++++++
MAINTAINERS | 3 +
drivers/platform/x86/intel/Makefile | 2 +-
drivers/platform/x86/intel/sdsi.c | 317 ++++++++++++++++----
drivers/platform/x86/intel/sdsi.h | 47 +++
drivers/platform/x86/intel/sdsi_genl.c | 249 +++++++++++++++
include/uapi/linux/intel-sdsi.h | 40 +++
tools/arch/x86/intel_sdsi/intel_sdsi.c | 99 +++---
8 files changed, 754 insertions(+), 100 deletions(-)
create mode 100644 Documentation/netlink/specs/intel_sdsi.yaml
create mode 100644 drivers/platform/x86/intel/sdsi.h
create mode 100644 drivers/platform/x86/intel/sdsi_genl.c
create mode 100644 include/uapi/linux/intel-sdsi.h
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
@ 2024-02-01 1:07 ` David E. Box
2024-02-01 16:49 ` Kuppuswamy Sathyanarayanan
` (2 more replies)
2024-02-01 1:07 ` [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows David E. Box
` (8 subsequent siblings)
9 siblings, 3 replies; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
New mailbox commands will support sending multi packet writes and updated
firmware now requires that the message size be written for all commands
along with the packet size. Since the driver doesn't perform writes larger
than the packet size, set the message size to the same value.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/platform/x86/intel/sdsi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 556e7c6dbb05..a70c071de6e2 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -252,6 +252,7 @@ static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *in
FIELD_PREP(CTRL_SOM, 1) |
FIELD_PREP(CTRL_RUN_BUSY, 1) |
FIELD_PREP(CTRL_READ_WRITE, 1) |
+ FIELD_PREP(CTRL_MSG_SIZE, info->size) |
FIELD_PREP(CTRL_PACKET_SIZE, info->size);
writeq(control, priv->control_addr);
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
2024-02-01 1:07 ` [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes David E. Box
@ 2024-02-01 1:07 ` David E. Box
2024-02-01 17:31 ` Kuppuswamy Sathyanarayanan
2024-02-08 13:38 ` Ilpo Järvinen
2024-02-01 1:07 ` [PATCH 3/8] platform/x86/intel/sdsi: Add header file David E. Box
` (7 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
The current mailbox commands are either read-only or write-only and the
flow is different for each. New commands will need to send and receive
data. In preparation for these commands, create a common polling function
to handle sending data and receiving in the same transaction.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/platform/x86/intel/sdsi.c | 79 +++++++++++++++++--------------
1 file changed, 44 insertions(+), 35 deletions(-)
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index a70c071de6e2..05a35f2f85b6 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -15,6 +15,7 @@
#include <linux/iopoll.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/overflow.h>
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/sysfs.h>
@@ -156,8 +157,8 @@ static int sdsi_status_to_errno(u32 status)
}
}
-static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
- size_t *data_size)
+static int sdsi_mbox_poll(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
+ size_t *data_size)
{
struct device *dev = priv->dev;
u32 total, loop, eom, status, message_size;
@@ -166,18 +167,10 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
lockdep_assert_held(&priv->mb_lock);
- /* Format and send the read command */
- control = FIELD_PREP(CTRL_EOM, 1) |
- FIELD_PREP(CTRL_SOM, 1) |
- FIELD_PREP(CTRL_RUN_BUSY, 1) |
- FIELD_PREP(CTRL_PACKET_SIZE, info->size);
- writeq(control, priv->control_addr);
-
/* For reads, data sizes that are larger than the mailbox size are read in packets. */
total = 0;
loop = 0;
do {
- void *buf = info->buffer + (SDSI_SIZE_MAILBOX * loop);
u32 packet_size;
/* Poll on ready bit */
@@ -195,6 +188,11 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
if (ret)
break;
+ if (!packet_size) {
+ sdsi_complete_transaction(priv);
+ break;
+ }
+
/* Only the last packet can be less than the mailbox size. */
if (!eom && packet_size != SDSI_SIZE_MAILBOX) {
dev_err(dev, "Invalid packet size\n");
@@ -208,9 +206,13 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
break;
}
- sdsi_memcpy64_fromio(buf, priv->mbox_addr, round_up(packet_size, SDSI_SIZE_CMD));
+ if (packet_size && info->buffer) {
+ void *buf = info->buffer + array_size(SDSI_SIZE_MAILBOX, loop);
- total += packet_size;
+ sdsi_memcpy64_fromio(buf, priv->mbox_addr,
+ round_up(packet_size, SDSI_SIZE_CMD));
+ total += packet_size;
+ }
sdsi_complete_transaction(priv);
} while (!eom && ++loop < MBOX_MAX_PACKETS);
@@ -230,16 +232,33 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
dev_warn(dev, "Read count %u differs from expected count %u\n",
total, message_size);
- *data_size = total;
+ if (data_size)
+ *data_size = total;
return 0;
}
-static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info)
+static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
+ size_t *data_size)
+{
+ u64 control;
+
+ lockdep_assert_held(&priv->mb_lock);
+
+ /* Format and send the read command */
+ control = FIELD_PREP(CTRL_EOM, 1) |
+ FIELD_PREP(CTRL_SOM, 1) |
+ FIELD_PREP(CTRL_RUN_BUSY, 1) |
+ FIELD_PREP(CTRL_PACKET_SIZE, info->size);
+ writeq(control, priv->control_addr);
+
+ return sdsi_mbox_poll(priv, info, data_size);
+}
+
+static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
+ size_t *data_size)
{
u64 control;
- u32 status;
- int ret;
lockdep_assert_held(&priv->mb_lock);
@@ -256,20 +275,7 @@ static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *in
FIELD_PREP(CTRL_PACKET_SIZE, info->size);
writeq(control, priv->control_addr);
- /* Poll on ready bit */
- ret = readq_poll_timeout(priv->control_addr, control, control & CTRL_READY,
- MBOX_POLLING_PERIOD_US, MBOX_TIMEOUT_US);
-
- if (ret)
- goto release_mbox;
-
- status = FIELD_GET(CTRL_STATUS, control);
- ret = sdsi_status_to_errno(status);
-
-release_mbox:
- sdsi_complete_transaction(priv);
-
- return ret;
+ return sdsi_mbox_poll(priv, info, data_size);
}
static int sdsi_mbox_acquire(struct sdsi_priv *priv, struct sdsi_mbox_info *info)
@@ -313,7 +319,8 @@ static int sdsi_mbox_acquire(struct sdsi_priv *priv, struct sdsi_mbox_info *info
return ret;
}
-static int sdsi_mbox_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info)
+static int sdsi_mbox_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
+ size_t *data_size)
{
int ret;
@@ -323,7 +330,7 @@ static int sdsi_mbox_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info)
if (ret)
return ret;
- return sdsi_mbox_cmd_write(priv, info);
+ return sdsi_mbox_cmd_write(priv, info, data_size);
}
static int sdsi_mbox_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info, size_t *data_size)
@@ -342,7 +349,7 @@ static int sdsi_mbox_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info, s
static ssize_t sdsi_provision(struct sdsi_priv *priv, char *buf, size_t count,
enum sdsi_command command)
{
- struct sdsi_mbox_info info;
+ struct sdsi_mbox_info info = {};
int ret;
if (count > (SDSI_SIZE_WRITE_MSG - SDSI_SIZE_CMD))
@@ -364,7 +371,9 @@ static ssize_t sdsi_provision(struct sdsi_priv *priv, char *buf, size_t count,
ret = mutex_lock_interruptible(&priv->mb_lock);
if (ret)
goto free_payload;
- ret = sdsi_mbox_write(priv, &info);
+
+ ret = sdsi_mbox_write(priv, &info, NULL);
+
mutex_unlock(&priv->mb_lock);
free_payload:
@@ -408,7 +417,7 @@ static ssize_t
certificate_read(u64 command, struct sdsi_priv *priv, char *buf, loff_t off,
size_t count)
{
- struct sdsi_mbox_info info;
+ struct sdsi_mbox_info info = {};
size_t size;
int ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/8] platform/x86/intel/sdsi: Add header file
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
2024-02-01 1:07 ` [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes David E. Box
2024-02-01 1:07 ` [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows David E. Box
@ 2024-02-01 1:07 ` David E. Box
2024-02-08 13:41 ` Ilpo Järvinen
2024-02-08 21:52 ` Kuppuswamy Sathyanarayanan
2024-02-01 1:07 ` [PATCH 4/8] platform/x86/intel/sdsi: Add netlink SPDM transport David E. Box
` (6 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
In preparation for new source files, move common structures to a new
header flie.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
MAINTAINERS | 1 +
drivers/platform/x86/intel/sdsi.c | 23 +----------------------
drivers/platform/x86/intel/sdsi.h | 31 +++++++++++++++++++++++++++++++
3 files changed, 33 insertions(+), 22 deletions(-)
create mode 100644 drivers/platform/x86/intel/sdsi.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 8d1052fa6a69..09ef8497e48a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11042,6 +11042,7 @@ INTEL SDSI DRIVER
M: David E. Box <david.e.box@linux.intel.com>
S: Supported
F: drivers/platform/x86/intel/sdsi.c
+F: drivers/platform/x86/intel/sdsi.h
F: tools/arch/x86/intel_sdsi/
F: tools/testing/selftests/drivers/sdsi/
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 05a35f2f85b6..d48bb648f0b2 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -22,24 +22,16 @@
#include <linux/types.h>
#include <linux/uaccess.h>
+#include "sdsi.h"
#include "vsec.h"
#define ACCESS_TYPE_BARID 2
#define ACCESS_TYPE_LOCAL 3
#define SDSI_MIN_SIZE_DWORDS 276
-#define SDSI_SIZE_MAILBOX 1024
#define SDSI_SIZE_REGS 80
#define SDSI_SIZE_CMD sizeof(u64)
-/*
- * Write messages are currently up to the size of the mailbox
- * while read messages are up to 4 times the size of the
- * mailbox, sent in packets
- */
-#define SDSI_SIZE_WRITE_MSG SDSI_SIZE_MAILBOX
-#define SDSI_SIZE_READ_MSG (SDSI_SIZE_MAILBOX * 4)
-
#define SDSI_ENABLED_FEATURES_OFFSET 16
#define SDSI_FEATURE_SDSI BIT(3)
#define SDSI_FEATURE_METERING BIT(26)
@@ -103,19 +95,6 @@ struct disc_table {
u32 offset;
};
-struct sdsi_priv {
- struct mutex mb_lock; /* Mailbox access lock */
- struct device *dev;
- 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;
-};
-
/* SDSi mailbox operations must be performed using 64bit mov instructions */
static __always_inline void
sdsi_memcpy64_toio(u64 __iomem *to, const u64 *from, size_t count_bytes)
diff --git a/drivers/platform/x86/intel/sdsi.h b/drivers/platform/x86/intel/sdsi.h
new file mode 100644
index 000000000000..d0d7450c7b2b
--- /dev/null
+++ b/drivers/platform/x86/intel/sdsi.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PDx86_SDSI_H_
+#define __PDx86_SDSI_H_
+#include <linux/mutex.h>
+#include <linux/types.h>
+
+#define SDSI_SIZE_MAILBOX 1024
+
+/*
+ * Write messages are currently up to the size of the mailbox
+ * while read messages are up to 4 times the size of the
+ * mailbox, sent in packets
+ */
+#define SDSI_SIZE_WRITE_MSG SDSI_SIZE_MAILBOX
+#define SDSI_SIZE_READ_MSG (SDSI_SIZE_MAILBOX * 4)
+
+struct device;
+
+struct sdsi_priv {
+ struct mutex mb_lock; /* Mailbox access lock */
+ struct device *dev;
+ 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;
+};
+#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/8] platform/x86/intel/sdsi: Add netlink SPDM transport
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
` (2 preceding siblings ...)
2024-02-01 1:07 ` [PATCH 3/8] platform/x86/intel/sdsi: Add header file David E. Box
@ 2024-02-01 1:07 ` David E. Box
2024-02-01 9:26 ` Jiri Pirko
2024-02-01 1:07 ` [PATCH 5/8] platform/x86/intel/sdsi: Add in-band BIOS lock support David E. Box
` (5 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
Intel On Demand adds attestation and firmware measurement retrieval
services through use of the protocols defined the Security Protocols and
Data Measurement (SPDM) specification. SPDM messages exchanges are used to
authenticate On Demand hardware and to retrieve signed measurements of the
NVRAM state used to track feature provisioning and the NVRAM state used for
metering services. These allow software to verify the authenticity of the
On Demand hardware as well as the integrity of the reported silicon
configuration.
Add a netlink SPDM transport for sending SPDM messages through the On
Demand mailbox. Provides commands to get a list of SPDM enabled devices,
get the message size limits for SPDM Requesters and Responders, and perform
an SPDM message exchange.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Tested-by: Wendy Wang <wendy.wang@intel.com>
Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.0.1.pdf [1]
---
Documentation/netlink/specs/intel_sdsi.yaml | 97 ++++++++
MAINTAINERS | 2 +
drivers/platform/x86/intel/Makefile | 2 +-
drivers/platform/x86/intel/sdsi.c | 164 ++++++++++++-
drivers/platform/x86/intel/sdsi.h | 14 ++
drivers/platform/x86/intel/sdsi_genl.c | 249 ++++++++++++++++++++
include/uapi/linux/intel-sdsi.h | 40 ++++
7 files changed, 565 insertions(+), 3 deletions(-)
create mode 100644 Documentation/netlink/specs/intel_sdsi.yaml
create mode 100644 drivers/platform/x86/intel/sdsi_genl.c
create mode 100644 include/uapi/linux/intel-sdsi.h
diff --git a/Documentation/netlink/specs/intel_sdsi.yaml b/Documentation/netlink/specs/intel_sdsi.yaml
new file mode 100644
index 000000000000..eeeaaffe3e81
--- /dev/null
+++ b/Documentation/netlink/specs/intel_sdsi.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause)
+
+name: intel-sdsi
+
+protocol: genetlink
+
+doc:
+ Intel On Demand generic netlink ABI for Attestation servives.
+
+attribute-sets:
+ -
+ name: sdsi
+ name-prefix: sdsi-genl-attr-
+ attributes:
+ -
+ name: unspec
+ type: unused
+ value: 0
+ -
+ name: devs
+ doc: List of On Demand entries
+ type: nest
+ multi-attr: true
+ nested-attributes: dev-info
+ -
+ name: dev-id
+ type: u32
+ -
+ name: dev-name
+ type: string
+ -
+ name: spdm-req
+ type: binary
+ -
+ name: spdm-rsp
+ type: binary
+ -
+ name: spdm-rsp-size
+ type: u32
+ -
+ name: spdm-req-size
+ type: u32
+ -
+ name: dev-info
+ subset-of: sdsi
+ attributes:
+ -
+ name: dev-id
+ type: u32
+ -
+ name: dev-name
+ type: string
+
+operations:
+ name-prefix: sdsi-genl-cmd-
+ list:
+ -
+ name: unspec
+ doc: unused
+ value: 0
+ -
+ name: get-devs
+ doc: Returns a list of available On Demand entries
+ attribute-set: sdsi
+ do: &get-devs-op
+ request:
+ attributes:
+ - dev-id
+ reply:
+ attributes:
+ - devs
+ dump: *get-devs-op
+ -
+ name: get-info
+ doc: Returns information about On Demand devices
+ attribute-set: sdsi
+ do:
+ request:
+ attributes:
+ - dev-id
+ reply:
+ attributes:
+ - spdm-req-size
+ - spdm-rsp-size
+ -
+ name: get-spdm
+ doc: Send and receive SPDM messages
+ attribute-set: sdsi
+ do:
+ request:
+ attributes:
+ - dev-id
+ - spdm-req
+ reply:
+ attributes:
+ - dev-id
+ - spdm-rsp
diff --git a/MAINTAINERS b/MAINTAINERS
index 09ef8497e48a..523943140bf5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11041,8 +11041,10 @@ F: drivers/platform/x86/intel_scu_*
INTEL SDSI DRIVER
M: David E. Box <david.e.box@linux.intel.com>
S: Supported
+F: drivers/platform/x86/intel/sdsi_genl.c
F: drivers/platform/x86/intel/sdsi.c
F: drivers/platform/x86/intel/sdsi.h
+F: include/uapi/linux/sdsi_nl.h
F: tools/arch/x86/intel_sdsi/
F: tools/testing/selftests/drivers/sdsi/
diff --git a/drivers/platform/x86/intel/Makefile b/drivers/platform/x86/intel/Makefile
index c1d5fe05e3f3..e1408d60d6ea 100644
--- a/drivers/platform/x86/intel/Makefile
+++ b/drivers/platform/x86/intel/Makefile
@@ -28,7 +28,7 @@ intel_int0002_vgpio-y := int0002_vgpio.o
obj-$(CONFIG_INTEL_INT0002_VGPIO) += intel_int0002_vgpio.o
intel_oaktrail-y := oaktrail.o
obj-$(CONFIG_INTEL_OAKTRAIL) += intel_oaktrail.o
-intel_sdsi-y := sdsi.o
+intel_sdsi-y := sdsi.o sdsi_genl.o
obj-$(CONFIG_INTEL_SDSI) += intel_sdsi.o
intel_vsec-y := vsec.o
obj-$(CONFIG_INTEL_VSEC) += intel_vsec.o
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index d48bb648f0b2..14821fee249c 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -11,6 +11,7 @@
#include <linux/auxiliary_bus.h>
#include <linux/bits.h>
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/iopoll.h>
#include <linux/kernel.h>
@@ -34,6 +35,7 @@
#define SDSI_ENABLED_FEATURES_OFFSET 16
#define SDSI_FEATURE_SDSI BIT(3)
+#define SDSI_FEATURE_ATTESTATION BIT(12)
#define SDSI_FEATURE_METERING BIT(26)
#define SDSI_SOCKET_ID_OFFSET 64
@@ -76,11 +78,15 @@
#define GUID_V2_CNTRL_SIZE 16
#define GUID_V2_REGS_SIZE 80
+LIST_HEAD(sdsi_list);
+DEFINE_MUTEX(sdsi_list_lock);
+
enum sdsi_command {
SDSI_CMD_PROVISION_AKC = 0x0004,
SDSI_CMD_PROVISION_CAP = 0x0008,
SDSI_CMD_READ_STATE = 0x0010,
SDSI_CMD_READ_METER = 0x0014,
+ SDSI_CMD_ATTESTATION = 0x1012,
};
struct sdsi_mbox_info {
@@ -533,6 +539,99 @@ static const struct attribute_group sdsi_group = {
};
__ATTRIBUTE_GROUPS(sdsi);
+bool sdsi_supports_attestation(struct sdsi_priv *priv)
+{
+ return priv->features & SDSI_FEATURE_ATTESTATION;
+}
+
+/* SPDM transport */
+int sdsi_spdm_exchange(void *private, const void *request, size_t request_sz,
+ void *response, size_t response_sz)
+{
+ struct sdsi_priv *priv = private;
+ struct sdsi_mbox_info info = {};
+ size_t spdm_msg_size, size;
+ int ret;
+ u64 *payload __free(kfree) = NULL;
+
+ /*
+ * For the attestation command, the mailbox write size is the sum of:
+ * Size of the SPDM request payload, padded for qword alignment
+ * 8 bytes for the mailbox command
+ * 8 bytes for the actual (non-padded) size of the SPDM request
+ */
+ if (request_sz > (SDSI_SIZE_WRITE_MSG - (2 * sizeof(u64))))
+ return -EOVERFLOW;
+
+ info.size = round_up(request_sz, sizeof(u64)) + 2 * sizeof(u64);
+
+ payload = kzalloc(info.size, GFP_KERNEL);
+ if (!payload)
+ return -ENOMEM;
+
+ memcpy(payload, request, request_sz);
+
+ /* The non-padded SPDM payload size is the 2nd-to-last qword */
+ payload[(info.size / sizeof(u64)) - 2] = request_sz;
+
+ /* Attestation mailbox command is the last qword of payload buffer */
+ payload[(info.size / sizeof(u64)) - 1] = SDSI_CMD_ATTESTATION;
+
+ info.payload = payload;
+ info.buffer = response;
+
+ ret = mutex_lock_interruptible(&priv->mb_lock);
+ if (ret)
+ return ret;
+ ret = sdsi_mbox_write(priv, &info, &size);
+ mutex_unlock(&priv->mb_lock);
+
+ if (ret < 0)
+ return ret;
+
+ /*
+ * The read size is the sum of:
+ * Size of the SPDM response payload, padded for qword alignment
+ * 8 bytes for the actual (non-padded) size of the SPDM payload
+ */
+
+ if (size < sizeof(u64)) {
+ dev_err(priv->dev,
+ "Attestation error: Mailbox reply size, %ld, too small\n",
+ size);
+ return -EPROTO;
+ }
+
+ if (!IS_ALIGNED(size, sizeof(u64))) {
+ dev_err(priv->dev,
+ "Attestation error: Mailbox reply size, %ld, is not aligned\n",
+ size);
+ return -EPROTO;
+ }
+
+ /*
+ * Get the SPDM response size from the last QWORD and check it fits
+ * with no more than 7 bytes of padding
+ */
+ spdm_msg_size = ((u64 *)info.buffer)[(size - sizeof(u64)) / sizeof(u64)];
+ if (!in_range(size - spdm_msg_size - sizeof(u64), 0, 8)) {
+ dev_err(priv->dev,
+ "Attestation error: Invalid SPDM response size, %ld\n",
+ spdm_msg_size);
+ return -EPROTO;
+ }
+
+ if (spdm_msg_size > response_sz) {
+ dev_err(priv->dev, "Attestation error: Expected response size %ld, got %ld\n",
+ response_sz, spdm_msg_size);
+ return -EOVERFLOW;
+ }
+
+ memcpy(response, info.buffer, spdm_msg_size);
+
+ return spdm_msg_size;
+}
+
static int sdsi_get_layout(struct sdsi_priv *priv, struct disc_table *table)
{
switch (table->guid) {
@@ -614,6 +713,7 @@ static int sdsi_probe(struct auxiliary_device *auxdev, const struct auxiliary_de
return -ENOMEM;
priv->dev = &auxdev->dev;
+ priv->id = auxdev->id;
mutex_init(&priv->mb_lock);
auxiliary_set_drvdata(auxdev, priv);
@@ -637,9 +737,36 @@ static int sdsi_probe(struct auxiliary_device *auxdev, const struct auxiliary_de
if (ret)
return ret;
+ mutex_lock(&sdsi_list_lock);
+ list_add(&priv->node, &sdsi_list);
+ mutex_unlock(&sdsi_list_lock);
+
return 0;
}
+static void sdsi_remove(struct auxiliary_device *auxdev)
+{
+ struct sdsi_priv *priv = auxiliary_get_drvdata(auxdev);
+
+ list_del(&priv->node);
+}
+
+struct sdsi_priv *sdsi_dev_get_by_id(int id)
+{
+ struct sdsi_priv *priv, *match = NULL;
+
+ mutex_lock(&sdsi_list_lock);
+ list_for_each_entry(priv, &sdsi_list, node) {
+ if (priv->id == id) {
+ match = priv;
+ break;
+ }
+ }
+ mutex_unlock(&sdsi_list_lock);
+
+ return match;
+}
+
static const struct auxiliary_device_id sdsi_aux_id_table[] = {
{ .name = "intel_vsec.sdsi" },
{}
@@ -652,9 +779,42 @@ static struct auxiliary_driver sdsi_aux_driver = {
},
.id_table = sdsi_aux_id_table,
.probe = sdsi_probe,
- /* No remove. All resources are handled under devm */
+ .remove = sdsi_remove,
};
-module_auxiliary_driver(sdsi_aux_driver);
+
+static bool netlink_initialized;
+
+static int __init sdsi_init(void)
+{
+ int ret;
+
+ ret = auxiliary_driver_register(&sdsi_aux_driver);
+ if (ret)
+ goto error;
+
+ if (sdsi_netlink_init())
+ pr_warn("Intel SDSi failed to init netlink\n");
+ else
+ netlink_initialized = true;
+
+ return 0;
+
+error:
+ mutex_destroy(&sdsi_list_lock);
+ return ret;
+}
+module_init(sdsi_init);
+
+static void __exit sdsi_exit(void)
+{
+ if (netlink_initialized)
+ sdsi_netlink_exit();
+
+ auxiliary_driver_unregister(&sdsi_aux_driver);
+
+ mutex_destroy(&sdsi_list_lock);
+}
+module_exit(sdsi_exit);
MODULE_AUTHOR("David E. Box <david.e.box@linux.intel.com>");
MODULE_DESCRIPTION("Intel On Demand (SDSi) driver");
diff --git a/drivers/platform/x86/intel/sdsi.h b/drivers/platform/x86/intel/sdsi.h
index d0d7450c7b2b..256618eb3136 100644
--- a/drivers/platform/x86/intel/sdsi.h
+++ b/drivers/platform/x86/intel/sdsi.h
@@ -19,13 +19,27 @@ struct device;
struct sdsi_priv {
struct mutex mb_lock; /* Mailbox access lock */
struct device *dev;
+ struct intel_vsec_device *ivdev;
+ struct list_head node;
void __iomem *control_addr;
void __iomem *mbox_addr;
void __iomem *regs_addr;
int control_size;
int maibox_size;
int registers_size;
+ int id;
u32 guid;
u32 features;
};
+
+extern struct list_head sdsi_list;
+extern struct mutex sdsi_list_lock;
+
+extern bool sdsi_supports_attestation(struct sdsi_priv *priv);
+extern int
+sdsi_spdm_exchange(void *private, const void *request, size_t request_sz,
+ void *response, size_t response_sz);
+extern struct sdsi_priv *sdsi_dev_get_by_id(int id);
+extern int sdsi_netlink_init(void);
+extern int sdsi_netlink_exit(void);
#endif
diff --git a/drivers/platform/x86/intel/sdsi_genl.c b/drivers/platform/x86/intel/sdsi_genl.c
new file mode 100644
index 000000000000..bca1671eba0d
--- /dev/null
+++ b/drivers/platform/x86/intel/sdsi_genl.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023, Intel Corporation.
+ * All Rights Reserved.
+ *
+ * Author: David E. Box <david.e.box@linux.intel.com>
+ *
+ * Netlink ABI for Intel On Demand SPDM transport
+ */
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/intel-sdsi.h>
+#include <net/genetlink.h>
+
+#include "sdsi.h"
+
+static struct genl_family sdsi_nl_family;
+
+static const struct nla_policy sdsi_genl_policy[SDSI_GENL_ATTR_MAX + 1] = {
+ [SDSI_GENL_ATTR_DEVS] = { .type = NLA_NESTED },
+ [SDSI_GENL_ATTR_DEV_ID] = { .type = NLA_U32 },
+ [SDSI_GENL_ATTR_DEV_NAME] = { .type = NLA_STRING },
+ [SDSI_GENL_ATTR_SPDM_REQ] = { .type = NLA_BINARY },
+ [SDSI_GENL_ATTR_SPDM_RSP] = { .type = NLA_BINARY },
+ [SDSI_GENL_ATTR_SPDM_REQ_SIZE] = { .type = NLA_U32 },
+ [SDSI_GENL_ATTR_SPDM_RSP_SIZE] = { .type = NLA_U32 },
+};
+
+struct param {
+ struct nlattr **attrs;
+ struct sk_buff *msg;
+ struct sdsi_priv *priv;
+};
+
+typedef int (*sdsi_genl_cb_t)(struct param *);
+
+static int sdsi_genl_cmd_spdm(struct param *p)
+{
+ struct sk_buff *msg = p->msg;
+ struct sdsi_priv *priv = p->priv;
+ void *response __free(kfree) = NULL;
+ void *request;
+ int rsp_size, req_size;
+ int ret;
+
+ if (!sdsi_supports_attestation(priv))
+ return -EOPNOTSUPP;
+
+ if (!p->attrs[SDSI_GENL_ATTR_SPDM_REQ])
+ return -EINVAL;
+
+ request = nla_data(p->attrs[SDSI_GENL_ATTR_SPDM_REQ]);
+ req_size = nla_len(p->attrs[SDSI_GENL_ATTR_SPDM_REQ]);
+
+ response = kmalloc(SDSI_SIZE_READ_MSG, GFP_KERNEL);
+ if (!response)
+ return -ENOMEM;
+
+ rsp_size = sdsi_spdm_exchange(priv, request, req_size, response,
+ SDSI_SIZE_READ_MSG);
+ if (rsp_size < 0)
+ return rsp_size;
+
+ ret = nla_put_u32(msg, SDSI_GENL_ATTR_DEV_ID, priv->id);
+ if (ret)
+ return ret;
+
+ return nla_put(msg, SDSI_GENL_ATTR_SPDM_RSP, rsp_size,
+ response);
+}
+
+static int sdsi_genl_cmd_get_devs(struct param *p)
+{
+ struct sk_buff *msg = p->msg;
+ struct nlattr *nest_start;
+ struct sdsi_priv *priv = p->priv;
+
+ nest_start = nla_nest_start(msg, SDSI_GENL_ATTR_DEVS);
+ if (!nest_start)
+ return -EMSGSIZE;
+
+ if (nla_put_u32(msg, SDSI_GENL_ATTR_DEV_ID, priv->id) ||
+ nla_put_string(msg, SDSI_GENL_ATTR_DEV_NAME, dev_name(priv->dev)))
+ goto out_cancel_nest;
+
+ nla_nest_end(msg, nest_start);
+
+ return 0;
+
+out_cancel_nest:
+ nla_nest_cancel(msg, nest_start);
+
+ return -EMSGSIZE;
+}
+
+static int sdsi_genl_cmd_get_info(struct param *p)
+{
+ struct sk_buff *msg = p->msg;
+ int ret;
+
+ ret = nla_put_u32(msg, SDSI_GENL_ATTR_SPDM_REQ_SIZE,
+ SDSI_SIZE_WRITE_MSG - (2 * sizeof(u64)));
+ if (ret)
+ return ret;
+
+ return nla_put_u32(msg, SDSI_GENL_ATTR_SPDM_RSP_SIZE,
+ SDSI_SIZE_READ_MSG - (sizeof(u64)));
+}
+
+static sdsi_genl_cb_t sdsi_genl_cmd_cb[] = {
+ [SDSI_GENL_CMD_GET_DEVS] = sdsi_genl_cmd_get_devs,
+ [SDSI_GENL_CMD_GET_INFO] = sdsi_genl_cmd_get_info,
+ [SDSI_GENL_CMD_GET_SPDM] = sdsi_genl_cmd_spdm,
+};
+
+static int sdsi_genl_cmd_dumpit(struct sk_buff *skb,
+ struct netlink_callback *cb)
+{
+ struct param p = { .msg = skb };
+ struct sdsi_priv *entry;
+ const struct genl_dumpit_info *info = genl_dumpit_info(cb);
+ int cmd = info->op.cmd;
+ int ret = 0, idx = 0;
+ void *hdr;
+
+ hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
+ &sdsi_nl_family, NLM_F_MULTI, cmd);
+ if (!hdr)
+ return -EMSGSIZE;
+
+ mutex_lock(&sdsi_list_lock);
+ list_for_each_entry(entry, &sdsi_list, node) {
+ p.priv = entry;
+ ret = sdsi_genl_cmd_cb[cmd](&p);
+ if (ret)
+ break;
+ idx++;
+ }
+ mutex_unlock(&sdsi_list_lock);
+
+ if (ret)
+ goto out_cancel_msg;
+
+ genlmsg_end(skb, hdr);
+
+ return 0;
+
+out_cancel_msg:
+ genlmsg_cancel(skb, hdr);
+ return ret;
+}
+
+static int sdsi_genl_cmd_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct param p = { .attrs = info->attrs };
+ struct sdsi_priv *priv, *entry;
+ struct sk_buff *msg;
+ void *hdr;
+ int cmd = info->genlhdr->cmd;
+ int ret = 0;
+ int id;
+
+ if (!p.attrs[SDSI_GENL_ATTR_DEV_ID])
+ return -EINVAL;
+
+ id = nla_get_u32(p.attrs[SDSI_GENL_ATTR_DEV_ID]);
+
+ priv = sdsi_dev_get_by_id(id);
+ if (!priv)
+ return -ENODEV;
+
+ msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ p.msg = msg;
+ p.priv = priv;
+
+ hdr = genlmsg_put_reply(msg, info, &sdsi_nl_family, 0, cmd);
+ if (!hdr)
+ goto out_free_msg;
+
+ mutex_lock(&sdsi_list_lock);
+ list_for_each_entry(entry, &sdsi_list, node) {
+ if (entry == priv) {
+ ret = sdsi_genl_cmd_cb[cmd](&p);
+ if (ret)
+ break;
+ break;
+ }
+ }
+ mutex_unlock(&sdsi_list_lock);
+
+ if (ret)
+ goto out_cancel_msg;
+
+ genlmsg_end(msg, hdr);
+
+ return genlmsg_reply(msg, info);
+
+out_cancel_msg:
+ genlmsg_cancel(msg, hdr);
+out_free_msg:
+ nlmsg_free(msg);
+
+ return ret;
+}
+
+static const struct genl_ops sdsi_genl_ops[] = {
+ {
+ .cmd = SDSI_GENL_CMD_GET_DEVS,
+ .doit = sdsi_genl_cmd_doit,
+ .dumpit = sdsi_genl_cmd_dumpit,
+ },
+ {
+ .cmd = SDSI_GENL_CMD_GET_INFO,
+ .doit = sdsi_genl_cmd_doit,
+ .flags = GENL_ADMIN_PERM,
+ },
+ {
+ .cmd = SDSI_GENL_CMD_GET_SPDM,
+ .doit = sdsi_genl_cmd_doit,
+ .flags = GENL_ADMIN_PERM,
+ },
+};
+
+static struct genl_family sdsi_nl_family __ro_after_init = {
+ .hdrsize = 0,
+ .name = SDSI_FAMILY_NAME,
+ .version = SDSI_FAMILY_VERSION,
+ .maxattr = SDSI_GENL_ATTR_MAX,
+ .policy = sdsi_genl_policy,
+ .ops = sdsi_genl_ops,
+ .resv_start_op = SDSI_GENL_CMD_MAX + 1,
+ .n_ops = ARRAY_SIZE(sdsi_genl_ops),
+};
+
+int __init sdsi_netlink_init(void)
+{
+ return genl_register_family(&sdsi_nl_family);
+}
+
+int sdsi_netlink_exit(void)
+{
+ return genl_unregister_family(&sdsi_nl_family);
+}
diff --git a/include/uapi/linux/intel-sdsi.h b/include/uapi/linux/intel-sdsi.h
new file mode 100644
index 000000000000..db7c49a66fdd
--- /dev/null
+++ b/include/uapi/linux/intel-sdsi.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Intel On Demand SPDM Interface
+ * Copyright (c) 2023, Intel Corporation.
+ * All rights reserved.
+ *
+ * Author: David E. Box <david.e.box@linux.intel.com>
+ */
+
+#ifndef __SDSI_NL_H
+#define __SDSI_NL_H
+
+#define SDSI_FAMILY_NAME "intel_sdsi"
+#define SDSI_FAMILY_VERSION 1
+
+enum {
+ SDSI_GENL_ATTR_UNSPEC,
+ SDSI_GENL_ATTR_DEVS, /* nested */
+ SDSI_GENL_ATTR_DEV_ID, /* u32, device id */
+ SDSI_GENL_ATTR_DEV_NAME, /* string, device name */
+ SDSI_GENL_ATTR_SPDM_REQ, /* binary, SDPM request message */
+ SDSI_GENL_ATTR_SPDM_RSP, /* binary, SDPM response message */
+ SDSI_GENL_ATTR_SPDM_REQ_SIZE, /* u32, max SDPM request size */
+ SDSI_GENL_ATTR_SPDM_RSP_SIZE, /* u32, max SPDM response size */
+
+ __SDSI_GENL_ATTR_MAX,
+ SDSI_GENL_ATTR_MAX = (__SDSI_GENL_ATTR_MAX - 1)
+};
+
+enum {
+ SDSI_GENL_CMD_UNSPEC,
+ SDSI_GENL_CMD_GET_DEVS, /* Get On Demand device list */
+ SDSI_GENL_CMD_GET_INFO, /* Get On Demand device info */
+ SDSI_GENL_CMD_GET_SPDM, /* Get SPDM response to SPDM request */
+
+ __SDSI_GENL_CMD_MAX,
+ SDSI_GENL_CMD_MAX = (__SDSI_GENL_CMD_MAX - 1)
+};
+
+#endif /* __SDSI_NL_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/8] platform/x86/intel/sdsi: Add in-band BIOS lock support
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
` (3 preceding siblings ...)
2024-02-01 1:07 ` [PATCH 4/8] platform/x86/intel/sdsi: Add netlink SPDM transport David E. Box
@ 2024-02-01 1:07 ` David E. Box
2024-02-08 13:52 ` Ilpo Järvinen
2024-02-01 1:07 ` [PATCH 6/8] platform/x86/intel/sdsi: Add attribute to read the current meter state David E. Box
` (4 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
As per SDSi in-band interface specification, sec titled "BIOS lock for
in-band provisioning", when IB_LOCK bit is set in control qword, the
SDSI agent is only allowed to perform the read flow, but not allowed to
provision license blob or license key. So add check for it in
sdsi_provision().
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/platform/x86/intel/sdsi.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 14821fee249c..287780fe65bb 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -61,6 +61,7 @@
#define CTRL_OWNER GENMASK(5, 4)
#define CTRL_COMPLETE BIT(6)
#define CTRL_READY BIT(7)
+#define CTRL_INBAND_LOCK BIT(32)
#define CTRL_STATUS GENMASK(15, 8)
#define CTRL_PACKET_SIZE GENMASK(31, 16)
#define CTRL_MSG_SIZE GENMASK(63, 48)
@@ -331,12 +332,21 @@ static int sdsi_mbox_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info, s
return sdsi_mbox_cmd_read(priv, info, data_size);
}
+static bool sdsi_ib_locked(struct sdsi_priv *priv)
+{
+ return !!FIELD_GET(CTRL_INBAND_LOCK, readq(priv->control_addr));
+}
+
static ssize_t sdsi_provision(struct sdsi_priv *priv, char *buf, size_t count,
enum sdsi_command command)
{
struct sdsi_mbox_info info = {};
int ret;
+ /* Make sure In-band lock is not set */
+ if (sdsi_ib_locked(priv))
+ return -EPERM;
+
if (count > (SDSI_SIZE_WRITE_MSG - SDSI_SIZE_CMD))
return -EOVERFLOW;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 6/8] platform/x86/intel/sdsi: Add attribute to read the current meter state
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
` (4 preceding siblings ...)
2024-02-01 1:07 ` [PATCH 5/8] platform/x86/intel/sdsi: Add in-band BIOS lock support David E. Box
@ 2024-02-01 1:07 ` David E. Box
2024-02-08 14:43 ` Ilpo Järvinen
2024-02-01 1:07 ` [PATCH 7/8] tools: Fix errors in meter_certificate display David E. Box
` (3 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
The meter_certificate file provides access to metering information that may
be attested but is only updated every 8 hours. Add new attribute,
meter_current, to allow reading an untested snapshot of the current values.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/platform/x86/intel/sdsi.c | 42 ++++++++++++++++++++++++++++---
drivers/platform/x86/intel/sdsi.h | 2 ++
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
index 287780fe65bb..171899b4a671 100644
--- a/drivers/platform/x86/intel/sdsi.c
+++ b/drivers/platform/x86/intel/sdsi.c
@@ -62,6 +62,7 @@
#define CTRL_COMPLETE BIT(6)
#define CTRL_READY BIT(7)
#define CTRL_INBAND_LOCK BIT(32)
+#define CTRL_METER_ENABLE_DRAM BIT(33)
#define CTRL_STATUS GENMASK(15, 8)
#define CTRL_PACKET_SIZE GENMASK(31, 16)
#define CTRL_MSG_SIZE GENMASK(63, 48)
@@ -235,8 +236,10 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
control = FIELD_PREP(CTRL_EOM, 1) |
FIELD_PREP(CTRL_SOM, 1) |
FIELD_PREP(CTRL_RUN_BUSY, 1) |
- FIELD_PREP(CTRL_PACKET_SIZE, info->size);
+ FIELD_PREP(CTRL_PACKET_SIZE, info->size) |
+ priv->control_flags;
writeq(control, priv->control_addr);
+ priv->control_flags = 0;
return sdsi_mbox_poll(priv, info, data_size);
}
@@ -468,11 +471,42 @@ meter_certificate_read(struct file *filp, struct kobject *kobj,
{
struct device *dev = kobj_to_dev(kobj);
struct sdsi_priv *priv = dev_get_drvdata(dev);
+ int ret;
- return certificate_read(SDSI_CMD_READ_METER, priv, buf, off, count);
+ ret = mutex_lock_interruptible(&priv->meter_lock);
+ if (ret)
+ return ret;
+
+ ret = certificate_read(SDSI_CMD_READ_METER, priv, buf, off, count);
+
+ mutex_unlock(&priv->meter_lock);
+
+ return ret;
}
static BIN_ATTR_ADMIN_RO(meter_certificate, SDSI_SIZE_READ_MSG);
+static ssize_t
+meter_current_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);
+ int ret;
+
+ ret = mutex_lock_interruptible(&priv->meter_lock);
+ if (ret)
+ return ret;
+
+ priv->control_flags = CTRL_METER_ENABLE_DRAM;
+ ret = certificate_read(SDSI_CMD_READ_METER, priv, buf, off, count);
+
+ mutex_unlock(&priv->meter_lock);
+
+ return ret;
+}
+static BIN_ATTR_ADMIN_RO(meter_current, 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)
@@ -503,6 +537,7 @@ static struct bin_attribute *sdsi_bin_attrs[] = {
&bin_attr_registers,
&bin_attr_state_certificate,
&bin_attr_meter_certificate,
+ &bin_attr_meter_current,
&bin_attr_provision_akc,
&bin_attr_provision_cap,
NULL
@@ -522,7 +557,7 @@ sdsi_battr_is_visible(struct kobject *kobj, struct bin_attribute *attr, int n)
if (!(priv->features & SDSI_FEATURE_SDSI))
return 0;
- if (attr == &bin_attr_meter_certificate)
+ if (attr == &bin_attr_meter_certificate || attr == &bin_attr_meter_current)
return (priv->features & SDSI_FEATURE_METERING) ?
attr->attr.mode : 0;
@@ -725,6 +760,7 @@ static int sdsi_probe(struct auxiliary_device *auxdev, const struct auxiliary_de
priv->dev = &auxdev->dev;
priv->id = auxdev->id;
mutex_init(&priv->mb_lock);
+ mutex_init(&priv->meter_lock);
auxiliary_set_drvdata(auxdev, priv);
/* Get the SDSi discovery table */
diff --git a/drivers/platform/x86/intel/sdsi.h b/drivers/platform/x86/intel/sdsi.h
index 256618eb3136..e20cf279212e 100644
--- a/drivers/platform/x86/intel/sdsi.h
+++ b/drivers/platform/x86/intel/sdsi.h
@@ -18,12 +18,14 @@ struct device;
struct sdsi_priv {
struct mutex mb_lock; /* Mailbox access lock */
+ struct mutex meter_lock;
struct device *dev;
struct intel_vsec_device *ivdev;
struct list_head node;
void __iomem *control_addr;
void __iomem *mbox_addr;
void __iomem *regs_addr;
+ u64 control_flags;
int control_size;
int maibox_size;
int registers_size;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 7/8] tools: Fix errors in meter_certificate display
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
` (5 preceding siblings ...)
2024-02-01 1:07 ` [PATCH 6/8] platform/x86/intel/sdsi: Add attribute to read the current meter state David E. Box
@ 2024-02-01 1:07 ` David E. Box
2024-02-08 14:46 ` Ilpo Järvinen
2024-02-01 1:07 ` [PATCH 8/8] tools: intel_sdsi: Add current meter support David E. Box
` (2 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
The maximum number of bundles in the meter certificate was hardcoded to
8 which caused extra bundles not to display. Instead, since the bundles
appear at the end of the file, set it to the remaining size from where
the bundles start.
Add missing 'version' field to struct meter_certificate.
Fix errors in the calculation of the start position of the counters and
in the display loop.
Fixes: aad129780bae ("platform/x86/intel/sdsi: Add support for reading the current meter state")
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
tools/arch/x86/intel_sdsi/intel_sdsi.c | 51 +++++++++++++++-----------
1 file changed, 30 insertions(+), 21 deletions(-)
diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
index 2cd92761f171..a8fb6d17405f 100644
--- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
+++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
@@ -43,7 +43,6 @@
#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)
@@ -154,11 +153,12 @@ struct bundle_encoding {
};
struct meter_certificate {
- uint32_t block_signature;
+ uint32_t signature;
+ uint32_t version;
+ uint64_t ppin;
uint32_t counter_unit;
- uint64_t ppin;
uint32_t bundle_length;
- uint32_t reserved;
+ uint64_t reserved;
uint32_t mmrc_encoding;
uint32_t mmrc_counter;
};
@@ -167,6 +167,9 @@ struct bundle_encoding_counter {
uint32_t encoding;
uint32_t counter;
};
+#define METER_MAX_NUM_BUNDLES \
+ (METER_CERT_MAX_SIZE - sizeof(struct meter_certificate) / \
+ sizeof(struct bundle_encoding_counter))
struct sdsi_dev {
struct sdsi_regs regs;
@@ -334,6 +337,7 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
uint32_t count = 0;
FILE *cert_ptr;
int ret, size;
+ char name[4];
ret = sdsi_update_registers(s);
if (ret)
@@ -375,32 +379,39 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
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);
+
+ get_feature(mc->signature, name);
+ printf("Signature: %.4s\n", name);
+
+ printf("Version: %d\n", mc->version);
+ printf("Count Unit: %dms\n", mc->counter_unit);
+ printf("PPIN: 0x%lx\n", mc->ppin);
+ printf("Feature Bundle Length: %d\n", mc->bundle_length);
+
+ get_feature(mc->mmrc_encoding, name);
+ printf("MMRC encoding: %.4s\n", name);
+
+ 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 than %d bundles: %d\n",
+ fprintf(stderr, "More than %ld bundles: actual %d\n",
METER_MAX_NUM_BUNDLES, mc->bundle_length / 8);
return -1;
}
- bec = (void *)(mc) + sizeof(mc);
+ 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];
+ printf("Number of Feature Counters: %d\n", mc->bundle_length / 8);
+ while (count < mc->bundle_length / 8) {
+ char feature[4];
- feature[4] = '\0';
get_feature(bec[count].encoding, feature);
- printf(" %s: %d\n", feature, bec[count].counter);
+ printf(" %.4s: %d\n", feature, bec[count].counter);
+ ++count;
}
return 0;
@@ -480,7 +491,7 @@ static int sdsi_state_cert_show(struct sdsi_dev *s)
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];
+ char feature[4];
uint32_t i;
printf(" Blob %d:\n", count - 1);
@@ -493,11 +504,9 @@ static int sdsi_state_cert_show(struct sdsi_dev *s)
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(" Feature %d: %.4s\n", i, feature);
}
if (lbc->num_bundles > STATE_MAX_NUM_IN_BUNDLE)
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 8/8] tools: intel_sdsi: Add current meter support
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
` (6 preceding siblings ...)
2024-02-01 1:07 ` [PATCH 7/8] tools: Fix errors in meter_certificate display David E. Box
@ 2024-02-01 1:07 ` David E. Box
2024-02-08 14:52 ` Ilpo Järvinen
2024-02-01 3:49 ` [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation Stephen Hemminger
2024-02-01 16:53 ` Kuppuswamy Sathyanarayanan
9 siblings, 1 reply; 27+ messages in thread
From: David E. Box @ 2024-02-01 1:07 UTC (permalink / raw)
To: netdev, ilpo.jarvinen, david.e.box, sathyanarayanan.kuppuswamy
Cc: linux-kernel, platform-driver-x86
Add support to read the 'meter_current' file. The display is the same as
the 'meter_certificate', but will show the current snapshot of the
counters.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
tools/arch/x86/intel_sdsi/intel_sdsi.c | 48 +++++++++++++++++---------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
index a8fb6d17405f..c9b3e457885d 100644
--- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
+++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
@@ -182,6 +182,7 @@ struct sdsi_dev {
enum command {
CMD_SOCKET_INFO,
CMD_METER_CERT,
+ CMD_METER_CURRENT_CERT,
CMD_STATE_CERT,
CMD_PROV_AKC,
CMD_PROV_CAP,
@@ -329,7 +330,7 @@ static void get_feature(uint32_t encoding, char *feature)
feature[0] = name[3];
}
-static int sdsi_meter_cert_show(struct sdsi_dev *s)
+static int sdsi_meter_cert_show(struct sdsi_dev *s, bool show_current)
{
char buf[METER_CERT_MAX_SIZE] = {0};
struct bundle_encoding_counter *bec;
@@ -360,7 +361,11 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
return ret;
}
- cert_ptr = fopen("meter_certificate", "r");
+ if (!show_current)
+ cert_ptr = fopen("meter_certificate", "r");
+ else
+ cert_ptr = fopen("meter_current", "r");
+
if (!cert_ptr) {
perror("Could not open 'meter_certificate' file");
return -1;
@@ -368,7 +373,8 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
size = fread(buf, 1, sizeof(buf), cert_ptr);
if (!size) {
- fprintf(stderr, "Could not read 'meter_certificate' file\n");
+ fprintf(stderr, "Could not read '%s' file\n",
+ show_current ? "meter_current" : "meter_certificate");
fclose(cert_ptr);
return -1;
}
@@ -734,7 +740,7 @@ static void sdsi_free_dev(struct sdsi_dev *s)
static void usage(char *prog)
{
- printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-m] [-a FILE] [-c FILE]]\n", prog);
+ printf("Usage: %s [-l] [-d DEVNO [-i] [-s] [-m | -C] [-a FILE] [-c FILE]\n", prog);
}
static void show_help(void)
@@ -743,8 +749,9 @@ static void show_help(void)
printf(" %-18s\t%s\n", "-l, --list", "list available On Demand devices");
printf(" %-18s\t%s\n", "-d, --devno DEVNO", "On Demand device number");
printf(" %-18s\t%s\n", "-i, --info", "show socket information");
- printf(" %-18s\t%s\n", "-s, --state", "show state certificate");
- printf(" %-18s\t%s\n", "-m, --meter", "show meter certificate");
+ printf(" %-18s\t%s\n", "-s, --state", "show state certificate data");
+ printf(" %-18s\t%s\n", "-m, --meter", "show meter certificate data");
+ printf(" %-18s\t%s\n", "-C, --meter_current", "show live unattested meter data");
printf(" %-18s\t%s\n", "-a, --akc FILE", "provision socket with AKC FILE");
printf(" %-18s\t%s\n", "-c, --cap FILE>", "provision socket with CAP FILE");
}
@@ -760,21 +767,22 @@ int main(int argc, char *argv[])
int option_index = 0;
static struct option long_options[] = {
- {"akc", required_argument, 0, 'a'},
- {"cap", required_argument, 0, 'c'},
- {"devno", required_argument, 0, 'd'},
- {"help", no_argument, 0, 'h'},
- {"info", no_argument, 0, 'i'},
- {"list", no_argument, 0, 'l'},
- {"meter", no_argument, 0, 'm'},
- {"state", no_argument, 0, 's'},
- {0, 0, 0, 0 }
+ {"akc", required_argument, 0, 'a'},
+ {"cap", required_argument, 0, 'c'},
+ {"devno", required_argument, 0, 'd'},
+ {"help", no_argument, 0, 'h'},
+ {"info", no_argument, 0, 'i'},
+ {"list", no_argument, 0, 'l'},
+ {"meter", no_argument, 0, 'm'},
+ {"meter_current", no_argument, 0, 'C'},
+ {"state", no_argument, 0, 's'},
+ {0, 0, 0, 0 }
};
progname = argv[0];
- while ((opt = getopt_long_only(argc, argv, "+a:c:d:hilms", long_options,
+ while ((opt = getopt_long_only(argc, argv, "+a:c:d:hilmCs", long_options,
&option_index)) != -1) {
switch (opt) {
case 'd':
@@ -790,6 +798,9 @@ int main(int argc, char *argv[])
case 'm':
command = CMD_METER_CERT;
break;
+ case 'C':
+ command = CMD_METER_CURRENT_CERT;
+ break;
case 's':
command = CMD_STATE_CERT;
break;
@@ -828,7 +839,10 @@ int main(int argc, char *argv[])
ret = sdsi_read_reg(s);
break;
case CMD_METER_CERT:
- ret = sdsi_meter_cert_show(s);
+ ret = sdsi_meter_cert_show(s, false);
+ break;
+ case CMD_METER_CURRENT_CERT:
+ ret = sdsi_meter_cert_show(s, true);
break;
case CMD_STATE_CERT:
ret = sdsi_state_cert_show(s);
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
` (7 preceding siblings ...)
2024-02-01 1:07 ` [PATCH 8/8] tools: intel_sdsi: Add current meter support David E. Box
@ 2024-02-01 3:49 ` Stephen Hemminger
2024-02-01 16:53 ` Kuppuswamy Sathyanarayanan
9 siblings, 0 replies; 27+ messages in thread
From: Stephen Hemminger @ 2024-02-01 3:49 UTC (permalink / raw)
To: David E. Box
Cc: netdev, ilpo.jarvinen, sathyanarayanan.kuppuswamy, linux-kernel,
platform-driver-x86
On Wed, 31 Jan 2024 17:07:39 -0800
"David E. Box" <david.e.box@linux.intel.com> wrote:
> This patch series primarily adds support for a new netlink ABI in the
> Intel On Demand driver for performing attestation of the hardware state.
Are there any tools (in iproute2) or tests to support this interface?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/8] platform/x86/intel/sdsi: Add netlink SPDM transport
2024-02-01 1:07 ` [PATCH 4/8] platform/x86/intel/sdsi: Add netlink SPDM transport David E. Box
@ 2024-02-01 9:26 ` Jiri Pirko
2024-02-01 16:42 ` David E. Box
0 siblings, 1 reply; 27+ messages in thread
From: Jiri Pirko @ 2024-02-01 9:26 UTC (permalink / raw)
To: David E. Box
Cc: netdev, ilpo.jarvinen, sathyanarayanan.kuppuswamy, linux-kernel,
platform-driver-x86
Thu, Feb 01, 2024 at 02:07:43AM CET, david.e.box@linux.intel.com wrote:
[...]
>+ -
>+ name: spdm-req
>+ type: binary
>+ -
>+ name: spdm-rsp
>+ type: binary
I don't understand the need to use netlink for this. Basically what you
do is you just use it to pass binary blobs to and from FW.
Advantages, like well-defined attributes, notifications etc, for which
it makes sense to use Netlink are not utilized at all.
Also, I don't thing it is good idea to have hw-driver-specific genl
family. I'm not aware of anything like that so far. Leave netlink
for use of generic and abstracted APIs.
Can't you just have a simple misc device for this?
Thanks.
>+ -
>+ name: spdm-rsp-size
>+ type: u32
>+ -
>+ name: spdm-req-size
>+ type: u32
>+ -
[...]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/8] platform/x86/intel/sdsi: Add netlink SPDM transport
2024-02-01 9:26 ` Jiri Pirko
@ 2024-02-01 16:42 ` David E. Box
2024-02-01 18:00 ` Jiri Pirko
0 siblings, 1 reply; 27+ messages in thread
From: David E. Box @ 2024-02-01 16:42 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, ilpo.jarvinen, sathyanarayanan.kuppuswamy, linux-kernel,
platform-driver-x86
Hi Jiro,
Thanks for your comments.
On Thu, 2024-02-01 at 10:26 +0100, Jiri Pirko wrote:
> Thu, Feb 01, 2024 at 02:07:43AM CET, david.e.box@linux.intel.com wrote:
>
> [...]
>
>
> > + -
> > + name: spdm-req
> > + type: binary
> > + -
> > + name: spdm-rsp
> > + type: binary
>
> I don't understand the need to use netlink for this. Basically what you
> do is you just use it to pass binary blobs to and from FW.
> Advantages, like well-defined attributes, notifications etc, for which
> it makes sense to use Netlink are not utilized at all.
SPDM supports the setup of a secure channel between the responder and requestor
using TLS based encryption algorthms. While this is just a transport for those
blobs, netlink seemed an appropriate interface for this type of communication.
The binary blobs can instead be broken out into the SPDM protocol messages,
right out of the spec. But for our needs this would still just define the
protocol. The algorithms themselves are not handled by the driver.
> Also, I don't thing it is good idea to have hw-driver-specific genl
> family. I'm not aware of anything like that so far. Leave netlink
> for use of generic and abstracted APIs.
Sounds like an implied rule. If so should it be documented somewhere?
>
> Can't you just have a simple misc device for this?
It wouldn't be too much work to convert it.
David
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes
2024-02-01 1:07 ` [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes David E. Box
@ 2024-02-01 16:49 ` Kuppuswamy Sathyanarayanan
2024-02-08 13:42 ` Ilpo Järvinen
2024-02-08 21:49 ` Kuppuswamy Sathyanarayanan
2 siblings, 0 replies; 27+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-02-01 16:49 UTC (permalink / raw)
To: David E. Box, netdev, ilpo.jarvinen; +Cc: linux-kernel, platform-driver-x86
On 1/31/24 5:07 PM, David E. Box wrote:
> New mailbox commands will support sending multi packet writes and updated
> firmware now requires that the message size be written for all commands
Can you include some spec reference to new mailbox commands?
What about updated firmware mean? Like a particular version?
> along with the packet size. Since the driver doesn't perform writes larger
> than the packet size, set the message size to the same value.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> drivers/platform/x86/intel/sdsi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 556e7c6dbb05..a70c071de6e2 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -252,6 +252,7 @@ static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *in
> FIELD_PREP(CTRL_SOM, 1) |
> FIELD_PREP(CTRL_RUN_BUSY, 1) |
> FIELD_PREP(CTRL_READ_WRITE, 1) |
> + FIELD_PREP(CTRL_MSG_SIZE, info->size) |
> FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> writeq(control, priv->control_addr);
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
` (8 preceding siblings ...)
2024-02-01 3:49 ` [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation Stephen Hemminger
@ 2024-02-01 16:53 ` Kuppuswamy Sathyanarayanan
2024-02-02 1:42 ` Jakub Kicinski
9 siblings, 1 reply; 27+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-02-01 16:53 UTC (permalink / raw)
To: David E. Box, netdev, ilpo.jarvinen; +Cc: linux-kernel, platform-driver-x86
On 1/31/24 5:07 PM, David E. Box wrote:
> This patch series primarily adds support for a new netlink ABI in the
> Intel On Demand driver for performing attestation of the hardware state.
Try to add some info about why you need new netlink ABI?
>
> Attestation patches
>
> Patch 1: The attestation mailbox command requires that the message size
> register be set along with the package size. Adds that support.
>
> Patch 2: The attestation command will need to write the SPDM message and
> read the response. The current mailbox flow handles reads and writes
> separately. Combines the two flows.
>
> Patch 3: Patch 4 will create a separate c file for the netlink
> interface. Add a separate header file now. No functional changes. This
> mostly just makes it easier to see the changes in Patch 4.
>
> Patch 4: Adds attestation support to the driver and provides a netlink
> interface to perform the service.
>
> Other changes
>
> Patch 5: Adds support to read the in-band BIOS lock. If set, On Demand
> controls are not available in the driver.
>
> Patch 6: Adds a new attribute to allow reading the most current metering
> state.
>
> Patch 7: Fixes for the intel_sdsi tool
>
> Patch 8: Adds support to the intel_sdsi tool to read the current meter
> state.
>
> David E. Box (7):
> platform/x86/intel/sdsi: Set message size during writes
> platform/x86/intel/sdsi: Combine read and write mailbox flows
> platform/x86/intel/sdsi: Add header file
> platform/x86/intel/sdsi: Add netlink SPDM transport
> platform/x86/intel/sdsi: Add attribute to read the current meter state
> tools: Fix errors in meter_certificate display
> tools: intel_sdsi: Add current meter support
>
> Kuppuswamy Sathyanarayanan (1):
> platform/x86/intel/sdsi: Add in-band BIOS lock support
>
> Documentation/netlink/specs/intel_sdsi.yaml | 97 ++++++
> MAINTAINERS | 3 +
> drivers/platform/x86/intel/Makefile | 2 +-
> drivers/platform/x86/intel/sdsi.c | 317 ++++++++++++++++----
> drivers/platform/x86/intel/sdsi.h | 47 +++
> drivers/platform/x86/intel/sdsi_genl.c | 249 +++++++++++++++
> include/uapi/linux/intel-sdsi.h | 40 +++
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 99 +++---
> 8 files changed, 754 insertions(+), 100 deletions(-)
> create mode 100644 Documentation/netlink/specs/intel_sdsi.yaml
> create mode 100644 drivers/platform/x86/intel/sdsi.h
> create mode 100644 drivers/platform/x86/intel/sdsi_genl.c
> create mode 100644 include/uapi/linux/intel-sdsi.h
>
>
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows
2024-02-01 1:07 ` [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows David E. Box
@ 2024-02-01 17:31 ` Kuppuswamy Sathyanarayanan
2024-02-01 18:11 ` David E. Box
2024-02-08 13:38 ` Ilpo Järvinen
1 sibling, 1 reply; 27+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-02-01 17:31 UTC (permalink / raw)
To: David E. Box, netdev, ilpo.jarvinen; +Cc: linux-kernel, platform-driver-x86
On 1/31/24 5:07 PM, David E. Box wrote:
> The current mailbox commands are either read-only or write-only and the
> flow is different for each. New commands will need to send and receive
> data. In preparation for these commands, create a common polling function
> to handle sending data and receiving in the same transaction.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> drivers/platform/x86/intel/sdsi.c | 79 +++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index a70c071de6e2..05a35f2f85b6 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -15,6 +15,7 @@
> #include <linux/iopoll.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/overflow.h>
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/sysfs.h>
> @@ -156,8 +157,8 @@ static int sdsi_status_to_errno(u32 status)
> }
> }
>
> -static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
> - size_t *data_size)
> +static int sdsi_mbox_poll(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
> + size_t *data_size)
> {
> struct device *dev = priv->dev;
> u32 total, loop, eom, status, message_size;
> @@ -166,18 +167,10 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
>
> lockdep_assert_held(&priv->mb_lock);
>
> - /* Format and send the read command */
> - control = FIELD_PREP(CTRL_EOM, 1) |
> - FIELD_PREP(CTRL_SOM, 1) |
> - FIELD_PREP(CTRL_RUN_BUSY, 1) |
> - FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> - writeq(control, priv->control_addr);
> -
> /* For reads, data sizes that are larger than the mailbox size are read in packets. */
> total = 0;
> loop = 0;
> do {
> - void *buf = info->buffer + (SDSI_SIZE_MAILBOX * loop);
> u32 packet_size;
>
> /* Poll on ready bit */
> @@ -195,6 +188,11 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
> if (ret)
> break;
>
> + if (!packet_size) {
> + sdsi_complete_transaction(priv);
> + break;
> + }
> +
It seems to be a generic check. Is this related to converting to a read/write function or
a common fix you added together in this patch.
> /* Only the last packet can be less than the mailbox size. */
> if (!eom && packet_size != SDSI_SIZE_MAILBOX) {
> dev_err(dev, "Invalid packet size\n");
> @@ -208,9 +206,13 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
> break;
> }
>
> - sdsi_memcpy64_fromio(buf, priv->mbox_addr, round_up(packet_size, SDSI_SIZE_CMD));
> + if (packet_size && info->buffer) {
> + void *buf = info->buffer + array_size(SDSI_SIZE_MAILBOX, loop);
>
> - total += packet_size;
> + sdsi_memcpy64_fromio(buf, priv->mbox_addr,
> + round_up(packet_size, SDSI_SIZE_CMD));
> + total += packet_size;
> + }
>
> sdsi_complete_transaction(priv);
> } while (!eom && ++loop < MBOX_MAX_PACKETS);
> @@ -230,16 +232,33 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
> dev_warn(dev, "Read count %u differs from expected count %u\n",
> total, message_size);
>
> - *data_size = total;
> + if (data_size)
> + *data_size = total;
>
> return 0;
> }
>
> -static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info)
> +static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
> + size_t *data_size)
> +{
> + u64 control;
> +
> + lockdep_assert_held(&priv->mb_lock);
> +
> + /* Format and send the read command */
> + control = FIELD_PREP(CTRL_EOM, 1) |
> + FIELD_PREP(CTRL_SOM, 1) |
> + FIELD_PREP(CTRL_RUN_BUSY, 1) |
> + FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> + writeq(control, priv->control_addr);
> +
> + return sdsi_mbox_poll(priv, info, data_size);
> +}
> +
> +static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
> + size_t *data_size)
> {
> u64 control;
> - u32 status;
> - int ret;
>
> lockdep_assert_held(&priv->mb_lock);
>
> @@ -256,20 +275,7 @@ static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *in
> FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> writeq(control, priv->control_addr);
>
> - /* Poll on ready bit */
> - ret = readq_poll_timeout(priv->control_addr, control, control & CTRL_READY,
> - MBOX_POLLING_PERIOD_US, MBOX_TIMEOUT_US);
> -
> - if (ret)
> - goto release_mbox;
> -
> - status = FIELD_GET(CTRL_STATUS, control);
> - ret = sdsi_status_to_errno(status);
> -
> -release_mbox:
> - sdsi_complete_transaction(priv);
> -
> - return ret;
> + return sdsi_mbox_poll(priv, info, data_size);
> }
>
> static int sdsi_mbox_acquire(struct sdsi_priv *priv, struct sdsi_mbox_info *info)
> @@ -313,7 +319,8 @@ static int sdsi_mbox_acquire(struct sdsi_priv *priv, struct sdsi_mbox_info *info
> return ret;
> }
>
> -static int sdsi_mbox_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info)
> +static int sdsi_mbox_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
> + size_t *data_size)
> {
> int ret;
>
> @@ -323,7 +330,7 @@ static int sdsi_mbox_write(struct sdsi_priv *priv, struct sdsi_mbox_info *info)
> if (ret)
> return ret;
>
> - return sdsi_mbox_cmd_write(priv, info);
> + return sdsi_mbox_cmd_write(priv, info, data_size);
> }
>
> static int sdsi_mbox_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info, size_t *data_size)
> @@ -342,7 +349,7 @@ static int sdsi_mbox_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info, s
> static ssize_t sdsi_provision(struct sdsi_priv *priv, char *buf, size_t count,
> enum sdsi_command command)
> {
> - struct sdsi_mbox_info info;
> + struct sdsi_mbox_info info = {};
This change also looks like an independent fix. Is this related to common function usage
you mentioned in the commit log.
> int ret;
>
> if (count > (SDSI_SIZE_WRITE_MSG - SDSI_SIZE_CMD))
> @@ -364,7 +371,9 @@ static ssize_t sdsi_provision(struct sdsi_priv *priv, char *buf, size_t count,
> ret = mutex_lock_interruptible(&priv->mb_lock);
> if (ret)
> goto free_payload;
> - ret = sdsi_mbox_write(priv, &info);
> +
> + ret = sdsi_mbox_write(priv, &info, NULL);
> +
> mutex_unlock(&priv->mb_lock);
>
> free_payload:
> @@ -408,7 +417,7 @@ static ssize_t
> certificate_read(u64 command, struct sdsi_priv *priv, char *buf, loff_t off,
> size_t count)
> {
> - struct sdsi_mbox_info info;
> + struct sdsi_mbox_info info = {};
> size_t size;
> int ret;
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/8] platform/x86/intel/sdsi: Add netlink SPDM transport
2024-02-01 16:42 ` David E. Box
@ 2024-02-01 18:00 ` Jiri Pirko
0 siblings, 0 replies; 27+ messages in thread
From: Jiri Pirko @ 2024-02-01 18:00 UTC (permalink / raw)
To: David E. Box
Cc: netdev, ilpo.jarvinen, sathyanarayanan.kuppuswamy, linux-kernel,
platform-driver-x86
Thu, Feb 01, 2024 at 05:42:33PM CET, david.e.box@linux.intel.com wrote:
>Hi Jiro,
>
>Thanks for your comments.
>
>On Thu, 2024-02-01 at 10:26 +0100, Jiri Pirko wrote:
>> Thu, Feb 01, 2024 at 02:07:43AM CET, david.e.box@linux.intel.com wrote:
>>
>> [...]
>>
>>
>> > + -
>> > + name: spdm-req
>> > + type: binary
>> > + -
>> > + name: spdm-rsp
>> > + type: binary
>>
>> I don't understand the need to use netlink for this. Basically what you
>> do is you just use it to pass binary blobs to and from FW.
>> Advantages, like well-defined attributes, notifications etc, for which
>> it makes sense to use Netlink are not utilized at all.
>
>SPDM supports the setup of a secure channel between the responder and requestor
>using TLS based encryption algorthms. While this is just a transport for those
>blobs, netlink seemed an appropriate interface for this type of communication.
>The binary blobs can instead be broken out into the SPDM protocol messages,
>right out of the spec. But for our needs this would still just define the
>protocol. The algorithms themselves are not handled by the driver.
If that is a standard, break it from blob into well-defined attributes
and push it out of your driver to some common code.
>
>> Also, I don't thing it is good idea to have hw-driver-specific genl
>> family. I'm not aware of anything like that so far. Leave netlink
>> for use of generic and abstracted APIs.
>
>Sounds like an implied rule. If so should it be documented somewhere?
>
>>
>> Can't you just have a simple misc device for this?
>
>It wouldn't be too much work to convert it.
>
>David
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows
2024-02-01 17:31 ` Kuppuswamy Sathyanarayanan
@ 2024-02-01 18:11 ` David E. Box
0 siblings, 0 replies; 27+ messages in thread
From: David E. Box @ 2024-02-01 18:11 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan, netdev, ilpo.jarvinen
Cc: linux-kernel, platform-driver-x86
Hi Sathya,
On Thu, 2024-02-01 at 09:31 -0800, Kuppuswamy Sathyanarayanan wrote:
>
> On 1/31/24 5:07 PM, David E. Box wrote:
> > The current mailbox commands are either read-only or write-only and the
> > flow is different for each. New commands will need to send and receive
> > data. In preparation for these commands, create a common polling function
> > to handle sending data and receiving in the same transaction.
> >
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > ---
> > drivers/platform/x86/intel/sdsi.c | 79 +++++++++++++++++--------------
> > 1 file changed, 44 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/platform/x86/intel/sdsi.c
> > b/drivers/platform/x86/intel/sdsi.c
> > index a70c071de6e2..05a35f2f85b6 100644
> > --- a/drivers/platform/x86/intel/sdsi.c
> > +++ b/drivers/platform/x86/intel/sdsi.c
> > @@ -15,6 +15,7 @@
> > #include <linux/iopoll.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > +#include <linux/overflow.h>
> > #include <linux/pci.h>
> > #include <linux/slab.h>
> > #include <linux/sysfs.h>
> > @@ -156,8 +157,8 @@ static int sdsi_status_to_errno(u32 status)
> > }
> > }
> >
> > -static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info
> > *info,
> > - size_t *data_size)
> > +static int sdsi_mbox_poll(struct sdsi_priv *priv, struct sdsi_mbox_info
> > *info,
> > + size_t *data_size)
> > {
> > struct device *dev = priv->dev;
> > u32 total, loop, eom, status, message_size;
> > @@ -166,18 +167,10 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv,
> > struct sdsi_mbox_info *inf
> >
> > lockdep_assert_held(&priv->mb_lock);
> >
> > - /* Format and send the read command */
> > - control = FIELD_PREP(CTRL_EOM, 1) |
> > - FIELD_PREP(CTRL_SOM, 1) |
> > - FIELD_PREP(CTRL_RUN_BUSY, 1) |
> > - FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> > - writeq(control, priv->control_addr);
> > -
> > /* For reads, data sizes that are larger than the mailbox size are
> > read in packets. */
> > total = 0;
> > loop = 0;
> > do {
> > - void *buf = info->buffer + (SDSI_SIZE_MAILBOX * loop);
> > u32 packet_size;
> >
> > /* Poll on ready bit */
> > @@ -195,6 +188,11 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv,
> > struct sdsi_mbox_info *inf
> > if (ret)
> > break;
> >
> > + if (!packet_size) {
> > + sdsi_complete_transaction(priv);
> > + break;
> > + }
> > +
>
> It seems to be a generic check. Is this related to converting to a read/write
> function or
> a common fix you added together in this patch.
Yes, it's related. The code that follows this only applies to reads. This check
would be true only for writes which only need to poll once to confirm the write
was successful. I'll add a comment to make it clear.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation
2024-02-01 16:53 ` Kuppuswamy Sathyanarayanan
@ 2024-02-02 1:42 ` Jakub Kicinski
0 siblings, 0 replies; 27+ messages in thread
From: Jakub Kicinski @ 2024-02-02 1:42 UTC (permalink / raw)
To: David E. Box
Cc: Kuppuswamy Sathyanarayanan, netdev, ilpo.jarvinen, linux-kernel,
platform-driver-x86
On Thu, 1 Feb 2024 08:53:37 -0800 Kuppuswamy Sathyanarayanan wrote:
> On 1/31/24 5:07 PM, David E. Box wrote:
> > This patch series primarily adds support for a new netlink ABI in the
> > Intel On Demand driver for performing attestation of the hardware state.
>
> Try to add some info about why you need new netlink ABI?
Since netdev is copied it'd also be useful to give us a high level
intro into what pieces are involved. Assume we have heard about
SPDM/attestation in context of NIC FW but have little understanding of
x86 platform stuff.
grep -i sdsi Documentation doesn't say much, the first Google result
for Intel On Demand reads like marketing fluff :(
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows
2024-02-01 1:07 ` [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows David E. Box
2024-02-01 17:31 ` Kuppuswamy Sathyanarayanan
@ 2024-02-08 13:38 ` Ilpo Järvinen
1 sibling, 0 replies; 27+ messages in thread
From: Ilpo Järvinen @ 2024-02-08 13:38 UTC (permalink / raw)
To: David E. Box
Cc: Netdev, sathyanarayanan.kuppuswamy, LKML, platform-driver-x86
On Wed, 31 Jan 2024, David E. Box wrote:
> The current mailbox commands are either read-only or write-only and the
> flow is different for each. New commands will need to send and receive
> data. In preparation for these commands, create a common polling function
> to handle sending data and receiving in the same transaction.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> drivers/platform/x86/intel/sdsi.c | 79 +++++++++++++++++--------------
> 1 file changed, 44 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index a70c071de6e2..05a35f2f85b6 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -15,6 +15,7 @@
> #include <linux/iopoll.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/overflow.h>
> #include <linux/pci.h>
> #include <linux/slab.h>
> #include <linux/sysfs.h>
> @@ -156,8 +157,8 @@ static int sdsi_status_to_errno(u32 status)
> }
> }
>
> -static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
> - size_t *data_size)
> +static int sdsi_mbox_poll(struct sdsi_priv *priv, struct sdsi_mbox_info *info,
> + size_t *data_size)
> {
> struct device *dev = priv->dev;
> u32 total, loop, eom, status, message_size;
> @@ -166,18 +167,10 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
>
> lockdep_assert_held(&priv->mb_lock);
>
> - /* Format and send the read command */
> - control = FIELD_PREP(CTRL_EOM, 1) |
> - FIELD_PREP(CTRL_SOM, 1) |
> - FIELD_PREP(CTRL_RUN_BUSY, 1) |
> - FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> - writeq(control, priv->control_addr);
> -
> /* For reads, data sizes that are larger than the mailbox size are read in packets. */
> total = 0;
> loop = 0;
> do {
> - void *buf = info->buffer + (SDSI_SIZE_MAILBOX * loop);
> u32 packet_size;
>
> /* Poll on ready bit */
> @@ -195,6 +188,11 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
> if (ret)
> break;
>
> + if (!packet_size) {
> + sdsi_complete_transaction(priv);
> + break;
> + }
> +
> /* Only the last packet can be less than the mailbox size. */
> if (!eom && packet_size != SDSI_SIZE_MAILBOX) {
> dev_err(dev, "Invalid packet size\n");
> @@ -208,9 +206,13 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
> break;
> }
>
> - sdsi_memcpy64_fromio(buf, priv->mbox_addr, round_up(packet_size, SDSI_SIZE_CMD));
> + if (packet_size && info->buffer) {
Why you check for packet_size here if you break earlier for !packet_size?
> + void *buf = info->buffer + array_size(SDSI_SIZE_MAILBOX, loop);
>
> - total += packet_size;
> + sdsi_memcpy64_fromio(buf, priv->mbox_addr,
> + round_up(packet_size, SDSI_SIZE_CMD));
> + total += packet_size;
> + }
>
> sdsi_complete_transaction(priv);
> } while (!eom && ++loop < MBOX_MAX_PACKETS);
> @@ -230,16 +232,33 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
> dev_warn(dev, "Read count %u differs from expected count %u\n",
> total, message_size);
>
> - *data_size = total;
> + if (data_size)
> + *data_size = total;
>
> return 0;
> }
--
i.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/8] platform/x86/intel/sdsi: Add header file
2024-02-01 1:07 ` [PATCH 3/8] platform/x86/intel/sdsi: Add header file David E. Box
@ 2024-02-08 13:41 ` Ilpo Järvinen
2024-02-08 21:52 ` Kuppuswamy Sathyanarayanan
1 sibling, 0 replies; 27+ messages in thread
From: Ilpo Järvinen @ 2024-02-08 13:41 UTC (permalink / raw)
To: David E. Box
Cc: Netdev, sathyanarayanan.kuppuswamy, LKML, platform-driver-x86
[-- Attachment #1: Type: text/plain, Size: 959 bytes --]
On Wed, 31 Jan 2024, David E. Box wrote:
> In preparation for new source files, move common structures to a new
> header flie.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> MAINTAINERS | 1 +
> drivers/platform/x86/intel/sdsi.c | 23 +----------------------
> drivers/platform/x86/intel/sdsi.h | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 33 insertions(+), 22 deletions(-)
> create mode 100644 drivers/platform/x86/intel/sdsi.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8d1052fa6a69..09ef8497e48a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11042,6 +11042,7 @@ INTEL SDSI DRIVER
> M: David E. Box <david.e.box@linux.intel.com>
> S: Supported
> F: drivers/platform/x86/intel/sdsi.c
> +F: drivers/platform/x86/intel/sdsi.h
Use this instead:
drivers/platform/x86/intel/sdsi*
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes
2024-02-01 1:07 ` [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes David E. Box
2024-02-01 16:49 ` Kuppuswamy Sathyanarayanan
@ 2024-02-08 13:42 ` Ilpo Järvinen
2024-02-08 21:49 ` Kuppuswamy Sathyanarayanan
2 siblings, 0 replies; 27+ messages in thread
From: Ilpo Järvinen @ 2024-02-08 13:42 UTC (permalink / raw)
To: David E. Box
Cc: Netdev, sathyanarayanan.kuppuswamy, LKML, platform-driver-x86
[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]
On Wed, 31 Jan 2024, David E. Box wrote:
> New mailbox commands will support sending multi packet writes and updated
> firmware now requires that the message size be written for all commands
> along with the packet size. Since the driver doesn't perform writes larger
> than the packet size, set the message size to the same value.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> drivers/platform/x86/intel/sdsi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 556e7c6dbb05..a70c071de6e2 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -252,6 +252,7 @@ static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *in
> FIELD_PREP(CTRL_SOM, 1) |
> FIELD_PREP(CTRL_RUN_BUSY, 1) |
> FIELD_PREP(CTRL_READ_WRITE, 1) |
> + FIELD_PREP(CTRL_MSG_SIZE, info->size) |
> FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> writeq(control, priv->control_addr);
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/8] platform/x86/intel/sdsi: Add in-band BIOS lock support
2024-02-01 1:07 ` [PATCH 5/8] platform/x86/intel/sdsi: Add in-band BIOS lock support David E. Box
@ 2024-02-08 13:52 ` Ilpo Järvinen
0 siblings, 0 replies; 27+ messages in thread
From: Ilpo Järvinen @ 2024-02-08 13:52 UTC (permalink / raw)
To: David E. Box
Cc: Netdev, sathyanarayanan.kuppuswamy, LKML, platform-driver-x86
On Wed, 31 Jan 2024, David E. Box wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> As per SDSi in-band interface specification, sec titled "BIOS lock for
> in-band provisioning", when IB_LOCK bit is set in control qword, the
> SDSI agent is only allowed to perform the read flow, but not allowed to
> provision license blob or license key. So add check for it in
> sdsi_provision().
>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> drivers/platform/x86/intel/sdsi.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 14821fee249c..287780fe65bb 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -61,6 +61,7 @@
> #define CTRL_OWNER GENMASK(5, 4)
> #define CTRL_COMPLETE BIT(6)
> #define CTRL_READY BIT(7)
> +#define CTRL_INBAND_LOCK BIT(32)
> #define CTRL_STATUS GENMASK(15, 8)
> #define CTRL_PACKET_SIZE GENMASK(31, 16)
> #define CTRL_MSG_SIZE GENMASK(63, 48)
> @@ -331,12 +332,21 @@ static int sdsi_mbox_read(struct sdsi_priv *priv, struct sdsi_mbox_info *info, s
> return sdsi_mbox_cmd_read(priv, info, data_size);
> }
>
> +static bool sdsi_ib_locked(struct sdsi_priv *priv)
> +{
> + return !!FIELD_GET(CTRL_INBAND_LOCK, readq(priv->control_addr));
> +}
> +
> static ssize_t sdsi_provision(struct sdsi_priv *priv, char *buf, size_t count,
> enum sdsi_command command)
> {
> struct sdsi_mbox_info info = {};
> int ret;
>
> + /* Make sure In-band lock is not set */
> + if (sdsi_ib_locked(priv))
> + return -EPERM;
> +
> if (count > (SDSI_SIZE_WRITE_MSG - SDSI_SIZE_CMD))
> return -EOVERFLOW;
Any reason why this order was chosen? I'd prefer these checks be other way
around (a stupid caller providing too long count gets notified of its
stupidity regardless of the locked state).
--
i.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/8] platform/x86/intel/sdsi: Add attribute to read the current meter state
2024-02-01 1:07 ` [PATCH 6/8] platform/x86/intel/sdsi: Add attribute to read the current meter state David E. Box
@ 2024-02-08 14:43 ` Ilpo Järvinen
0 siblings, 0 replies; 27+ messages in thread
From: Ilpo Järvinen @ 2024-02-08 14:43 UTC (permalink / raw)
To: David E. Box
Cc: Netdev, sathyanarayanan.kuppuswamy, LKML, platform-driver-x86
On Wed, 31 Jan 2024, David E. Box wrote:
> The meter_certificate file provides access to metering information that may
> be attested but is only updated every 8 hours. Add new attribute,
> meter_current, to allow reading an untested snapshot of the current values.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> drivers/platform/x86/intel/sdsi.c | 42 ++++++++++++++++++++++++++++---
> drivers/platform/x86/intel/sdsi.h | 2 ++
> 2 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 287780fe65bb..171899b4a671 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -62,6 +62,7 @@
> #define CTRL_COMPLETE BIT(6)
> #define CTRL_READY BIT(7)
> #define CTRL_INBAND_LOCK BIT(32)
> +#define CTRL_METER_ENABLE_DRAM BIT(33)
> #define CTRL_STATUS GENMASK(15, 8)
> #define CTRL_PACKET_SIZE GENMASK(31, 16)
> #define CTRL_MSG_SIZE GENMASK(63, 48)
> @@ -235,8 +236,10 @@ static int sdsi_mbox_cmd_read(struct sdsi_priv *priv, struct sdsi_mbox_info *inf
> control = FIELD_PREP(CTRL_EOM, 1) |
> FIELD_PREP(CTRL_SOM, 1) |
> FIELD_PREP(CTRL_RUN_BUSY, 1) |
> - FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> + FIELD_PREP(CTRL_PACKET_SIZE, info->size) |
> + priv->control_flags;
> writeq(control, priv->control_addr);
> + priv->control_flags = 0;
I'm slightly worried about this. The function is named with a generic
name but I suppose meter_lock that has less generic name is supposed to
protect this also?
Also, resetting it after every use smells like it should be a parameter
instead of struct member.
> return sdsi_mbox_poll(priv, info, data_size);
> }
> @@ -468,11 +471,42 @@ meter_certificate_read(struct file *filp, struct kobject *kobj,
> {
> struct device *dev = kobj_to_dev(kobj);
> struct sdsi_priv *priv = dev_get_drvdata(dev);
> + int ret;
>
> - return certificate_read(SDSI_CMD_READ_METER, priv, buf, off, count);
> + ret = mutex_lock_interruptible(&priv->meter_lock);
> + if (ret)
> + return ret;
> +
> + ret = certificate_read(SDSI_CMD_READ_METER, priv, buf, off, count);
> +
> + mutex_unlock(&priv->meter_lock);
> +
> + return ret;
> }
> static BIN_ATTR_ADMIN_RO(meter_certificate, SDSI_SIZE_READ_MSG);
>
> +static ssize_t
> +meter_current_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);
> + int ret;
> +
> + ret = mutex_lock_interruptible(&priv->meter_lock);
> + if (ret)
> + return ret;
> +
> + priv->control_flags = CTRL_METER_ENABLE_DRAM;
> + ret = certificate_read(SDSI_CMD_READ_METER, priv, buf, off, count);
> +
> + mutex_unlock(&priv->meter_lock);
> +
> + return ret;
> +}
> +static BIN_ATTR_ADMIN_RO(meter_current, 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)
> @@ -503,6 +537,7 @@ static struct bin_attribute *sdsi_bin_attrs[] = {
> &bin_attr_registers,
> &bin_attr_state_certificate,
> &bin_attr_meter_certificate,
> + &bin_attr_meter_current,
> &bin_attr_provision_akc,
> &bin_attr_provision_cap,
> NULL
> @@ -522,7 +557,7 @@ sdsi_battr_is_visible(struct kobject *kobj, struct bin_attribute *attr, int n)
> if (!(priv->features & SDSI_FEATURE_SDSI))
> return 0;
>
> - if (attr == &bin_attr_meter_certificate)
> + if (attr == &bin_attr_meter_certificate || attr == &bin_attr_meter_current)
> return (priv->features & SDSI_FEATURE_METERING) ?
> attr->attr.mode : 0;
>
> @@ -725,6 +760,7 @@ static int sdsi_probe(struct auxiliary_device *auxdev, const struct auxiliary_de
> priv->dev = &auxdev->dev;
> priv->id = auxdev->id;
> mutex_init(&priv->mb_lock);
> + mutex_init(&priv->meter_lock);
> auxiliary_set_drvdata(auxdev, priv);
>
> /* Get the SDSi discovery table */
> diff --git a/drivers/platform/x86/intel/sdsi.h b/drivers/platform/x86/intel/sdsi.h
> index 256618eb3136..e20cf279212e 100644
> --- a/drivers/platform/x86/intel/sdsi.h
> +++ b/drivers/platform/x86/intel/sdsi.h
> @@ -18,12 +18,14 @@ struct device;
>
> struct sdsi_priv {
> struct mutex mb_lock; /* Mailbox access lock */
> + struct mutex meter_lock;
Please add information what this protects.
--
i.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 7/8] tools: Fix errors in meter_certificate display
2024-02-01 1:07 ` [PATCH 7/8] tools: Fix errors in meter_certificate display David E. Box
@ 2024-02-08 14:46 ` Ilpo Järvinen
0 siblings, 0 replies; 27+ messages in thread
From: Ilpo Järvinen @ 2024-02-08 14:46 UTC (permalink / raw)
To: David E. Box
Cc: Netdev, sathyanarayanan.kuppuswamy, LKML, platform-driver-x86
On Wed, 31 Jan 2024, David E. Box wrote:
> The maximum number of bundles in the meter certificate was hardcoded to
> 8 which caused extra bundles not to display. Instead, since the bundles
> appear at the end of the file, set it to the remaining size from where
> the bundles start.
>
> Add missing 'version' field to struct meter_certificate.
>
> Fix errors in the calculation of the start position of the counters and
> in the display loop.
Why are all these bundled into a single commit? They sound like
independent changes.
--
i.
> Fixes: aad129780bae ("platform/x86/intel/sdsi: Add support for reading the current meter state")
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 51 +++++++++++++++-----------
> 1 file changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index 2cd92761f171..a8fb6d17405f 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -43,7 +43,6 @@
> #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)
> @@ -154,11 +153,12 @@ struct bundle_encoding {
> };
>
> struct meter_certificate {
> - uint32_t block_signature;
> + uint32_t signature;
> + uint32_t version;
> + uint64_t ppin;
> uint32_t counter_unit;
> - uint64_t ppin;
> uint32_t bundle_length;
> - uint32_t reserved;
> + uint64_t reserved;
> uint32_t mmrc_encoding;
> uint32_t mmrc_counter;
> };
> @@ -167,6 +167,9 @@ struct bundle_encoding_counter {
> uint32_t encoding;
> uint32_t counter;
> };
> +#define METER_MAX_NUM_BUNDLES \
> + (METER_CERT_MAX_SIZE - sizeof(struct meter_certificate) / \
> + sizeof(struct bundle_encoding_counter))
>
> struct sdsi_dev {
> struct sdsi_regs regs;
> @@ -334,6 +337,7 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
> uint32_t count = 0;
> FILE *cert_ptr;
> int ret, size;
> + char name[4];
>
> ret = sdsi_update_registers(s);
> if (ret)
> @@ -375,32 +379,39 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
> 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);
> +
> + get_feature(mc->signature, name);
> + printf("Signature: %.4s\n", name);
> +
> + printf("Version: %d\n", mc->version);
> + printf("Count Unit: %dms\n", mc->counter_unit);
> + printf("PPIN: 0x%lx\n", mc->ppin);
> + printf("Feature Bundle Length: %d\n", mc->bundle_length);
> +
> + get_feature(mc->mmrc_encoding, name);
> + printf("MMRC encoding: %.4s\n", name);
> +
> + 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 than %d bundles: %d\n",
> + fprintf(stderr, "More than %ld bundles: actual %d\n",
> METER_MAX_NUM_BUNDLES, mc->bundle_length / 8);
> return -1;
> }
>
> - bec = (void *)(mc) + sizeof(mc);
> + 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];
> + printf("Number of Feature Counters: %d\n", mc->bundle_length / 8);
> + while (count < mc->bundle_length / 8) {
> + char feature[4];
>
> - feature[4] = '\0';
> get_feature(bec[count].encoding, feature);
> - printf(" %s: %d\n", feature, bec[count].counter);
> + printf(" %.4s: %d\n", feature, bec[count].counter);
> + ++count;
> }
>
> return 0;
> @@ -480,7 +491,7 @@ static int sdsi_state_cert_show(struct sdsi_dev *s)
> 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];
> + char feature[4];
> uint32_t i;
>
> printf(" Blob %d:\n", count - 1);
> @@ -493,11 +504,9 @@ static int sdsi_state_cert_show(struct sdsi_dev *s)
> 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(" Feature %d: %.4s\n", i, feature);
> }
>
> if (lbc->num_bundles > STATE_MAX_NUM_IN_BUNDLE)
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 8/8] tools: intel_sdsi: Add current meter support
2024-02-01 1:07 ` [PATCH 8/8] tools: intel_sdsi: Add current meter support David E. Box
@ 2024-02-08 14:52 ` Ilpo Järvinen
0 siblings, 0 replies; 27+ messages in thread
From: Ilpo Järvinen @ 2024-02-08 14:52 UTC (permalink / raw)
To: David E. Box
Cc: Netdev, sathyanarayanan.kuppuswamy, LKML, platform-driver-x86
On Wed, 31 Jan 2024, David E. Box wrote:
> Add support to read the 'meter_current' file. The display is the same as
> the 'meter_certificate', but will show the current snapshot of the
> counters.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> tools/arch/x86/intel_sdsi/intel_sdsi.c | 48 +++++++++++++++++---------
> 1 file changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/tools/arch/x86/intel_sdsi/intel_sdsi.c b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> index a8fb6d17405f..c9b3e457885d 100644
> --- a/tools/arch/x86/intel_sdsi/intel_sdsi.c
> +++ b/tools/arch/x86/intel_sdsi/intel_sdsi.c
> @@ -182,6 +182,7 @@ struct sdsi_dev {
> enum command {
> CMD_SOCKET_INFO,
> CMD_METER_CERT,
> + CMD_METER_CURRENT_CERT,
> CMD_STATE_CERT,
> CMD_PROV_AKC,
> CMD_PROV_CAP,
> @@ -329,7 +330,7 @@ static void get_feature(uint32_t encoding, char *feature)
> feature[0] = name[3];
> }
>
> -static int sdsi_meter_cert_show(struct sdsi_dev *s)
> +static int sdsi_meter_cert_show(struct sdsi_dev *s, bool show_current)
> {
> char buf[METER_CERT_MAX_SIZE] = {0};
> struct bundle_encoding_counter *bec;
> @@ -360,7 +361,11 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
> return ret;
> }
>
> - cert_ptr = fopen("meter_certificate", "r");
> + if (!show_current)
> + cert_ptr = fopen("meter_certificate", "r");
> + else
> + cert_ptr = fopen("meter_current", "r");
Please calculate the filename only once:
cert_fname = show_current ? "meter_current" : "meter_certificate";
cert_ptr = fopen(cert_fname, "r");
> +
> if (!cert_ptr) {
> perror("Could not open 'meter_certificate' file");
This line still has 'meter_certificate' so you need to convert it to
fprintf() + strerror().
> return -1;
> @@ -368,7 +373,8 @@ static int sdsi_meter_cert_show(struct sdsi_dev *s)
>
> size = fread(buf, 1, sizeof(buf), cert_ptr);
> if (!size) {
> - fprintf(stderr, "Could not read 'meter_certificate' file\n");
> + fprintf(stderr, "Could not read '%s' file\n",
> + show_current ? "meter_current" : "meter_certificate");
Use the local variable from above.
--
i.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes
2024-02-01 1:07 ` [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes David E. Box
2024-02-01 16:49 ` Kuppuswamy Sathyanarayanan
2024-02-08 13:42 ` Ilpo Järvinen
@ 2024-02-08 21:49 ` Kuppuswamy Sathyanarayanan
2 siblings, 0 replies; 27+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-02-08 21:49 UTC (permalink / raw)
To: David E. Box, netdev, ilpo.jarvinen; +Cc: linux-kernel, platform-driver-x86
On 1/31/24 5:07 PM, David E. Box wrote:
> New mailbox commands will support sending multi packet writes and updated
> firmware now requires that the message size be written for all commands
> along with the packet size. Since the driver doesn't perform writes larger
> than the packet size, set the message size to the same value.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> drivers/platform/x86/intel/sdsi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 556e7c6dbb05..a70c071de6e2 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -252,6 +252,7 @@ static int sdsi_mbox_cmd_write(struct sdsi_priv *priv, struct sdsi_mbox_info *in
> FIELD_PREP(CTRL_SOM, 1) |
> FIELD_PREP(CTRL_RUN_BUSY, 1) |
> FIELD_PREP(CTRL_READ_WRITE, 1) |
> + FIELD_PREP(CTRL_MSG_SIZE, info->size) |
> FIELD_PREP(CTRL_PACKET_SIZE, info->size);
> writeq(control, priv->control_addr);
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/8] platform/x86/intel/sdsi: Add header file
2024-02-01 1:07 ` [PATCH 3/8] platform/x86/intel/sdsi: Add header file David E. Box
2024-02-08 13:41 ` Ilpo Järvinen
@ 2024-02-08 21:52 ` Kuppuswamy Sathyanarayanan
1 sibling, 0 replies; 27+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-02-08 21:52 UTC (permalink / raw)
To: David E. Box, netdev, ilpo.jarvinen; +Cc: linux-kernel, platform-driver-x86
On 1/31/24 5:07 PM, David E. Box wrote:
> In preparation for new source files, move common structures to a new
> header flie.
Add some detail about why you adding new source files.
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> MAINTAINERS | 1 +
> drivers/platform/x86/intel/sdsi.c | 23 +----------------------
> drivers/platform/x86/intel/sdsi.h | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 33 insertions(+), 22 deletions(-)
> create mode 100644 drivers/platform/x86/intel/sdsi.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8d1052fa6a69..09ef8497e48a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11042,6 +11042,7 @@ INTEL SDSI DRIVER
> M: David E. Box <david.e.box@linux.intel.com>
> S: Supported
> F: drivers/platform/x86/intel/sdsi.c
> +F: drivers/platform/x86/intel/sdsi.h
> F: tools/arch/x86/intel_sdsi/
> F: tools/testing/selftests/drivers/sdsi/
>
> diff --git a/drivers/platform/x86/intel/sdsi.c b/drivers/platform/x86/intel/sdsi.c
> index 05a35f2f85b6..d48bb648f0b2 100644
> --- a/drivers/platform/x86/intel/sdsi.c
> +++ b/drivers/platform/x86/intel/sdsi.c
> @@ -22,24 +22,16 @@
> #include <linux/types.h>
> #include <linux/uaccess.h>
>
> +#include "sdsi.h"
> #include "vsec.h"
>
> #define ACCESS_TYPE_BARID 2
> #define ACCESS_TYPE_LOCAL 3
>
> #define SDSI_MIN_SIZE_DWORDS 276
> -#define SDSI_SIZE_MAILBOX 1024
> #define SDSI_SIZE_REGS 80
> #define SDSI_SIZE_CMD sizeof(u64)
>
> -/*
> - * Write messages are currently up to the size of the mailbox
> - * while read messages are up to 4 times the size of the
> - * mailbox, sent in packets
> - */
> -#define SDSI_SIZE_WRITE_MSG SDSI_SIZE_MAILBOX
> -#define SDSI_SIZE_READ_MSG (SDSI_SIZE_MAILBOX * 4)
> -
> #define SDSI_ENABLED_FEATURES_OFFSET 16
> #define SDSI_FEATURE_SDSI BIT(3)
> #define SDSI_FEATURE_METERING BIT(26)
> @@ -103,19 +95,6 @@ struct disc_table {
> u32 offset;
> };
>
> -struct sdsi_priv {
> - struct mutex mb_lock; /* Mailbox access lock */
> - struct device *dev;
> - 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;
> -};
> -
> /* SDSi mailbox operations must be performed using 64bit mov instructions */
> static __always_inline void
> sdsi_memcpy64_toio(u64 __iomem *to, const u64 *from, size_t count_bytes)
> diff --git a/drivers/platform/x86/intel/sdsi.h b/drivers/platform/x86/intel/sdsi.h
> new file mode 100644
> index 000000000000..d0d7450c7b2b
> --- /dev/null
> +++ b/drivers/platform/x86/intel/sdsi.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __PDx86_SDSI_H_
> +#define __PDx86_SDSI_H_
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +
> +#define SDSI_SIZE_MAILBOX 1024
> +
> +/*
> + * Write messages are currently up to the size of the mailbox
> + * while read messages are up to 4 times the size of the
> + * mailbox, sent in packets
> + */
> +#define SDSI_SIZE_WRITE_MSG SDSI_SIZE_MAILBOX
> +#define SDSI_SIZE_READ_MSG (SDSI_SIZE_MAILBOX * 4)
> +
> +struct device;
> +
> +struct sdsi_priv {
> + struct mutex mb_lock; /* Mailbox access lock */
> + struct device *dev;
> + 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;
> +};
> +#endif
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2024-02-08 21:52 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-01 1:07 [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation David E. Box
2024-02-01 1:07 ` [PATCH 1/8] platform/x86/intel/sdsi: Set message size during writes David E. Box
2024-02-01 16:49 ` Kuppuswamy Sathyanarayanan
2024-02-08 13:42 ` Ilpo Järvinen
2024-02-08 21:49 ` Kuppuswamy Sathyanarayanan
2024-02-01 1:07 ` [PATCH 2/8] platform/x86/intel/sdsi: Combine read and write mailbox flows David E. Box
2024-02-01 17:31 ` Kuppuswamy Sathyanarayanan
2024-02-01 18:11 ` David E. Box
2024-02-08 13:38 ` Ilpo Järvinen
2024-02-01 1:07 ` [PATCH 3/8] platform/x86/intel/sdsi: Add header file David E. Box
2024-02-08 13:41 ` Ilpo Järvinen
2024-02-08 21:52 ` Kuppuswamy Sathyanarayanan
2024-02-01 1:07 ` [PATCH 4/8] platform/x86/intel/sdsi: Add netlink SPDM transport David E. Box
2024-02-01 9:26 ` Jiri Pirko
2024-02-01 16:42 ` David E. Box
2024-02-01 18:00 ` Jiri Pirko
2024-02-01 1:07 ` [PATCH 5/8] platform/x86/intel/sdsi: Add in-band BIOS lock support David E. Box
2024-02-08 13:52 ` Ilpo Järvinen
2024-02-01 1:07 ` [PATCH 6/8] platform/x86/intel/sdsi: Add attribute to read the current meter state David E. Box
2024-02-08 14:43 ` Ilpo Järvinen
2024-02-01 1:07 ` [PATCH 7/8] tools: Fix errors in meter_certificate display David E. Box
2024-02-08 14:46 ` Ilpo Järvinen
2024-02-01 1:07 ` [PATCH 8/8] tools: intel_sdsi: Add current meter support David E. Box
2024-02-08 14:52 ` Ilpo Järvinen
2024-02-01 3:49 ` [PATCH 0/8] Intel On Demand: Add netlink interface for SPDM attestation Stephen Hemminger
2024-02-01 16:53 ` Kuppuswamy Sathyanarayanan
2024-02-02 1:42 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).