From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
This is to support uni-directional transports such as SPDM over Storage.
As specified by the DMTF DSP0286.
Also update spdm_socket_rsp() to use the new send()/receive() functions. For
the case of spdm_socket_receive(), this allows us to do error checking
in one place with the addition of spdm_socket_command_valid().
Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
---
backends/spdm-socket.c | 56 +++++++++++++++++++++++++++++-------
include/system/spdm-socket.h | 35 ++++++++++++++++++++++
2 files changed, 80 insertions(+), 11 deletions(-)
diff --git a/backends/spdm-socket.c b/backends/spdm-socket.c
index 2c709c68c8..3d264814df 100644
--- a/backends/spdm-socket.c
+++ b/backends/spdm-socket.c
@@ -184,28 +184,62 @@ int spdm_socket_connect(uint16_t port, Error **errp)
return client_socket;
}
-uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
- void *req, uint32_t req_len,
- void *rsp, uint32_t rsp_len)
+static bool spdm_socket_command_valid(uint32_t command)
+{
+ switch (command) {
+ case SPDM_SOCKET_COMMAND_NORMAL:
+ case SPDM_SOCKET_STORAGE_CMD_IF_SEND:
+ case SPDM_SOCKET_STORAGE_CMD_IF_RECV:
+ case SOCKET_SPDM_STORAGE_ACK_STATUS:
+ case SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE:
+ case SPDM_SOCKET_COMMAND_CONTINUE:
+ case SPDM_SOCKET_COMMAND_SHUTDOWN:
+ case SPDM_SOCKET_COMMAND_UNKOWN:
+ case SPDM_SOCKET_COMMAND_TEST:
+ return true;
+ default:
+ return false;
+ }
+}
+
+uint32_t spdm_socket_receive(const int socket, uint32_t transport_type,
+ void *rsp, uint32_t rsp_len)
{
uint32_t command;
bool result;
- result = send_platform_data(socket, transport_type,
- SPDM_SOCKET_COMMAND_NORMAL,
- req, req_len);
- if (!result) {
+ result = receive_platform_data(socket, transport_type, &command,
+ (uint8_t *)rsp, &rsp_len);
+
+ /* we may have received some data, but check if the command is valid */
+ if (!result || !spdm_socket_command_valid(command)) {
return 0;
}
- result = receive_platform_data(socket, transport_type, &command,
- (uint8_t *)rsp, &rsp_len);
+ return rsp_len;
+}
+
+bool spdm_socket_send(const int socket, uint32_t socket_cmd,
+ uint32_t transport_type, void *req, uint32_t req_len)
+{
+ return send_platform_data(socket, transport_type,
+ socket_cmd, req, req_len);
+}
+
+uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
+ void *req, uint32_t req_len,
+ void *rsp, uint32_t rsp_len)
+{
+ bool result;
+
+ result = spdm_socket_send(socket, SPDM_SOCKET_COMMAND_NORMAL,
+ transport_type, req, req_len);
if (!result) {
return 0;
}
- assert(command != 0);
-
+ rsp_len = spdm_socket_receive(socket, transport_type, (uint8_t *)rsp,
+ rsp_len);
return rsp_len;
}
diff --git a/include/system/spdm-socket.h b/include/system/spdm-socket.h
index 5d8bd9aa4e..2b7d03f82d 100644
--- a/include/system/spdm-socket.h
+++ b/include/system/spdm-socket.h
@@ -50,6 +50,35 @@ uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
void *req, uint32_t req_len,
void *rsp, uint32_t rsp_len);
+/**
+ * spdm_socket_rsp: Receive a message from an SPDM server
+ * @socket: socket returned from spdm_socket_connect()
+ * @transport_type: SPDM_SOCKET_TRANSPORT_TYPE_* macro
+ * @rsp: response buffer
+ * @rsp_len: response buffer length
+ *
+ * Receives a message from the SPDM server and returns the number of bytes
+ * received or 0 on failure. This can be used to receive a message from the SPDM
+ * server without sending anything first.
+ */
+uint32_t spdm_socket_receive(const int socket, uint32_t transport_type,
+ void *rsp, uint32_t rsp_len);
+
+/**
+ * spdm_socket_rsp: Sends a message to an SPDM server
+ * @socket: socket returned from spdm_socket_connect()
+ * @socket_cmd: socket command type (normal/if_recv/if_send etc...)
+ * @transport_type: SPDM_SOCKET_TRANSPORT_TYPE_* macro
+ * @req: request buffer
+ * @req_len: request buffer length
+ *
+ * Sends platform data to a SPDM server on socket, returns true on success.
+ * The response from the server must then be fetched by using
+ * spdm_socket_receive().
+ */
+bool spdm_socket_send(const int socket, uint32_t socket_cmd,
+ uint32_t transport_type, void *req, uint32_t req_len);
+
/**
* spdm_socket_close: send a shutdown command to the server
* @socket: socket returned from spdm_socket_connect()
@@ -60,6 +89,9 @@ uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
void spdm_socket_close(const int socket, uint32_t transport_type);
#define SPDM_SOCKET_COMMAND_NORMAL 0x0001
+#define SPDM_SOCKET_STORAGE_CMD_IF_SEND 0x0002
+#define SPDM_SOCKET_STORAGE_CMD_IF_RECV 0x0003
+#define SOCKET_SPDM_STORAGE_ACK_STATUS 0x0004
#define SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE 0x8001
#define SPDM_SOCKET_COMMAND_CONTINUE 0xFFFD
#define SPDM_SOCKET_COMMAND_SHUTDOWN 0xFFFE
@@ -68,7 +100,10 @@ void spdm_socket_close(const int socket, uint32_t transport_type);
#define SPDM_SOCKET_TRANSPORT_TYPE_MCTP 0x01
#define SPDM_SOCKET_TRANSPORT_TYPE_PCI_DOE 0x02
+#define SPDM_SOCKET_TRANSPORT_TYPE_SCSI 0x03
+#define SPDM_SOCKET_TRANSPORT_TYPE_NVME 0x04
#define SPDM_SOCKET_MAX_MESSAGE_BUFFER_SIZE 0x1200
+#define SPDM_SOCKET_MAX_MSG_STATUS_LEN 0x02
#endif
--
2.51.0On Thu, 4 Sep 2025 13:10:55 +1000
Wilfred Mallawa <wilfred.opensource@gmail.com> wrote:
> From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
>
> This is to support uni-directional transports such as SPDM over Storage.
> As specified by the DMTF DSP0286.
>
> Also update spdm_socket_rsp() to use the new send()/receive() functions. For
> the case of spdm_socket_receive(), this allows us to do error checking
> in one place with the addition of spdm_socket_command_valid().
>
> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Hi Wilfred,
Main comments are focused on patch break up and a few definitions that
are here but not used yet. Move those to the patch that first uses them.
Jonathan
> ---
> backends/spdm-socket.c | 56 +++++++++++++++++++++++++++++-------
> include/system/spdm-socket.h | 35 ++++++++++++++++++++++
> 2 files changed, 80 insertions(+), 11 deletions(-)
>
> diff --git a/backends/spdm-socket.c b/backends/spdm-socket.c
> index 2c709c68c8..3d264814df 100644
> --- a/backends/spdm-socket.c
> +++ b/backends/spdm-socket.c
> @@ -184,28 +184,62 @@ int spdm_socket_connect(uint16_t port, Error **errp)
> return client_socket;
> }
>
> -uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
> - void *req, uint32_t req_len,
> - void *rsp, uint32_t rsp_len)
> +static bool spdm_socket_command_valid(uint32_t command)
As below - perhaps this sanity check belongs in a precursor patch?
> +{
> + switch (command) {
> + case SPDM_SOCKET_COMMAND_NORMAL:
> + case SPDM_SOCKET_STORAGE_CMD_IF_SEND:
> + case SPDM_SOCKET_STORAGE_CMD_IF_RECV:
> + case SOCKET_SPDM_STORAGE_ACK_STATUS:
> + case SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE:
> + case SPDM_SOCKET_COMMAND_CONTINUE:
> + case SPDM_SOCKET_COMMAND_SHUTDOWN:
> + case SPDM_SOCKET_COMMAND_UNKOWN:
> + case SPDM_SOCKET_COMMAND_TEST:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +uint32_t spdm_socket_receive(const int socket, uint32_t transport_type,
> + void *rsp, uint32_t rsp_len)
> {
> uint32_t command;
> bool result;
>
> - result = send_platform_data(socket, transport_type,
> - SPDM_SOCKET_COMMAND_NORMAL,
> - req, req_len);
> - if (!result) {
> + result = receive_platform_data(socket, transport_type, &command,
> + (uint8_t *)rsp, &rsp_len);
> +
> + /* we may have received some data, but check if the command is valid */
> + if (!result || !spdm_socket_command_valid(command)) {
Is this change related to the separate send/recv part? Perhaps it
is a useful bit of hardening to do as a precursor patch?
> return 0;
> }
>
> - result = receive_platform_data(socket, transport_type, &command,
> - (uint8_t *)rsp, &rsp_len);
> + return rsp_len;
> +}
> +
> +bool spdm_socket_send(const int socket, uint32_t socket_cmd,
> + uint32_t transport_type, void *req, uint32_t req_len)
> +{
> + return send_platform_data(socket, transport_type,
> + socket_cmd, req, req_len);
I'd wrap that closer to 80 chars.
> +}
> +
> +uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
> + void *req, uint32_t req_len,
> + void *rsp, uint32_t rsp_len)
> +{
> + bool result;
> +
> + result = spdm_socket_send(socket, SPDM_SOCKET_COMMAND_NORMAL,
> + transport_type, req, req_len);
> if (!result) {
> return 0;
> }
>
> - assert(command != 0);
> -
> + rsp_len = spdm_socket_receive(socket, transport_type, (uint8_t *)rsp,
Why casting to a uint8_t * ? It is a void * and this function takes a void *.
> + rsp_len);
> return rsp_len;
> }
>
> diff --git a/include/system/spdm-socket.h b/include/system/spdm-socket.h
> index 5d8bd9aa4e..2b7d03f82d 100644
> --- a/include/system/spdm-socket.h
> +++ b/include/system/spdm-socket.h
> /**
> * spdm_socket_close: send a shutdown command to the server
> * @socket: socket returned from spdm_socket_connect()
> @@ -60,6 +89,9 @@ uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
> void spdm_socket_close(const int socket, uint32_t transport_type);
>
> #define SPDM_SOCKET_COMMAND_NORMAL 0x0001
> +#define SPDM_SOCKET_STORAGE_CMD_IF_SEND 0x0002
> +#define SPDM_SOCKET_STORAGE_CMD_IF_RECV 0x0003
> +#define SOCKET_SPDM_STORAGE_ACK_STATUS 0x0004
> #define SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE 0x8001
> #define SPDM_SOCKET_COMMAND_CONTINUE 0xFFFD
> #define SPDM_SOCKET_COMMAND_SHUTDOWN 0xFFFE
> @@ -68,7 +100,10 @@ void spdm_socket_close(const int socket, uint32_t transport_type);
>
> #define SPDM_SOCKET_TRANSPORT_TYPE_MCTP 0x01
> #define SPDM_SOCKET_TRANSPORT_TYPE_PCI_DOE 0x02
> +#define SPDM_SOCKET_TRANSPORT_TYPE_SCSI 0x03
> +#define SPDM_SOCKET_TRANSPORT_TYPE_NVME 0x04
Not used in this patch. Move it to where it is first used.
>
> #define SPDM_SOCKET_MAX_MESSAGE_BUFFER_SIZE 0x1200
> +#define SPDM_SOCKET_MAX_MSG_STATUS_LEN 0x02
Not used in this patch.
>
> #endif
On Thu, 2025-09-04 at 11:10 +0100, Jonathan Cameron wrote:
> >
[...]
> > diff --git a/backends/spdm-socket.c b/backends/spdm-socket.c
> > index 2c709c68c8..3d264814df 100644
> > --- a/backends/spdm-socket.c
> > +++ b/backends/spdm-socket.c
> > @@ -184,28 +184,62 @@ int spdm_socket_connect(uint16_t port, Error
> > **errp)
> > return client_socket;
> > }
> >
> > -uint32_t spdm_socket_rsp(const int socket, uint32_t
> > transport_type,
> > - void *req, uint32_t req_len,
> > - void *rsp, uint32_t rsp_len)
> > +static bool spdm_socket_command_valid(uint32_t command)
> As below - perhaps this sanity check belongs in a precursor patch?
> > +{
> > + switch (command) {
> > + case SPDM_SOCKET_COMMAND_NORMAL:
> > + case SPDM_SOCKET_STORAGE_CMD_IF_SEND:
> > + case SPDM_SOCKET_STORAGE_CMD_IF_RECV:
> > + case SOCKET_SPDM_STORAGE_ACK_STATUS:
> > + case SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE:
> > + case SPDM_SOCKET_COMMAND_CONTINUE:
> > + case SPDM_SOCKET_COMMAND_SHUTDOWN:
> > + case SPDM_SOCKET_COMMAND_UNKOWN:
> > + case SPDM_SOCKET_COMMAND_TEST:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +uint32_t spdm_socket_receive(const int socket, uint32_t
> > transport_type,
> > + void *rsp, uint32_t rsp_len)
> > {
> > uint32_t command;
> > bool result;
> >
> > - result = send_platform_data(socket, transport_type,
> > - SPDM_SOCKET_COMMAND_NORMAL,
> > - req, req_len);
> > - if (!result) {
> > + result = receive_platform_data(socket, transport_type,
> > &command,
> > + (uint8_t *)rsp, &rsp_len);
> > +
> > + /* we may have received some data, but check if the command is
> > valid */
> > + if (!result || !spdm_socket_command_valid(command)) {
>
> Is this change related to the separate send/recv part? Perhaps it
> is a useful bit of hardening to do as a precursor patch?
Hey Jonathan,
I think it makes sense to integrate this directly into
spdm_socket_receive() in this patch. As in a precursor patch, it would
need to be changed here again.
>
> > return 0;
> > }
> >
> > - result = receive_platform_data(socket, transport_type,
> > &command,
> > - (uint8_t *)rsp, &rsp_len);
> > + return rsp_len;
> > +}
> > +
> > +bool spdm_socket_send(const int socket, uint32_t socket_cmd,
> > + uint32_t transport_type, void *req, uint32_t
> > req_len)
> > +{
> > + return send_platform_data(socket, transport_type,
> > + socket_cmd, req, req_len);
>
> I'd wrap that closer to 80 chars.
Ah yep!
>
> > +}
> > +
> > +uint32_t spdm_socket_rsp(const int socket, uint32_t
> > transport_type,
> > + void *req, uint32_t req_len,
> > + void *rsp, uint32_t rsp_len)
> > +{
> > + bool result;
> > +
> > + result = spdm_socket_send(socket, SPDM_SOCKET_COMMAND_NORMAL,
> > + transport_type, req, req_len);
> > if (!result) {
> > return 0;
> > }
> >
> > - assert(command != 0);
> > -
> > + rsp_len = spdm_socket_receive(socket, transport_type, (uint8_t
> > *)rsp,
>
> Why casting to a uint8_t * ? It is a void * and this function takes
> a void *.
Okay yeah, will fixup.
>
> > + rsp_len);
> > return rsp_len;
> > }
> >
> > diff --git a/include/system/spdm-socket.h b/include/system/spdm-
> > socket.h
> > index 5d8bd9aa4e..2b7d03f82d 100644
> > --- a/include/system/spdm-socket.h
> > +++ b/include/system/spdm-socket.h
>
> > /**
> > * spdm_socket_close: send a shutdown command to the server
> > * @socket: socket returned from spdm_socket_connect()
> > @@ -60,6 +89,9 @@ uint32_t spdm_socket_rsp(const int socket,
> > uint32_t transport_type,
> > void spdm_socket_close(const int socket, uint32_t transport_type);
> >
> > #define SPDM_SOCKET_COMMAND_NORMAL 0x0001
> > +#define SPDM_SOCKET_STORAGE_CMD_IF_SEND 0x0002
> > +#define SPDM_SOCKET_STORAGE_CMD_IF_RECV 0x0003
> > +#define SOCKET_SPDM_STORAGE_ACK_STATUS 0x0004
> > #define SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE 0x8001
> > #define SPDM_SOCKET_COMMAND_CONTINUE 0xFFFD
> > #define SPDM_SOCKET_COMMAND_SHUTDOWN 0xFFFE
> > @@ -68,7 +100,10 @@ void spdm_socket_close(const int socket,
> > uint32_t transport_type);
> >
> > #define SPDM_SOCKET_TRANSPORT_TYPE_MCTP 0x01
> > #define SPDM_SOCKET_TRANSPORT_TYPE_PCI_DOE 0x02
> > +#define SPDM_SOCKET_TRANSPORT_TYPE_SCSI 0x03
> > +#define SPDM_SOCKET_TRANSPORT_TYPE_NVME 0x04
>
> Not used in this patch. Move it to where it is first used.
>
> >
> > #define SPDM_SOCKET_MAX_MESSAGE_BUFFER_SIZE 0x1200
> > +#define SPDM_SOCKET_MAX_MSG_STATUS_LEN 0x02
>
> Not used in this patch.
Sounds good, I will move them up.
Thanks!
Wilfred
© 2016 - 2026 Red Hat, Inc.