| Message ID | 20250107052906.249973-5-wilfred.mallawa@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add SPDM over Storage transport support for NVMe | expand |
On Jan 7 15:29, Wilfred Mallawa via wrote: > This header contains the transport encoding for an SPDM message that > uses the SPDM over Storage transport as defined by the DMTF DSP0286. > > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> > --- > include/system/spdm-socket.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/include/system/spdm-socket.h b/include/system/spdm-socket.h > index 2b7d03f82d..fc007e5b48 100644 > --- a/include/system/spdm-socket.h > +++ b/include/system/spdm-socket.h > @@ -88,6 +88,18 @@ bool spdm_socket_send(const int socket, uint32_t socket_cmd, > */ > void spdm_socket_close(const int socket, uint32_t transport_type); > > +/* > + * Defines the transport encoding for SPDM, this information shall be passed > + * down to the SPDM server, when conforming to the SPDM over Storage standard > + * as defined by DSP0286. > + */ > +typedef struct QEMU_PACKED { > + uint8_t security_protocol; > + uint16_t security_protocol_specific; > + bool inc_512; > + uint32_t length; > +} StorageSpdmTransportHeader; Does it make sense to pack a bool? Is this defined by the SPDM server in use? I can't find the definition of this header anywhere. > + > #define SPDM_SOCKET_COMMAND_NORMAL 0x0001 > #define SPDM_SOCKET_STORAGE_CMD_IF_SEND 0x0002 > #define SPDM_SOCKET_STORAGE_CMD_IF_RECV 0x0003 > -- > 2.47.1 > >
On Fri, 2025-01-10 at 10:04 +0100, Klaus Jensen wrote: > On Jan 7 15:29, Wilfred Mallawa via wrote: > > This header contains the transport encoding for an SPDM message > > that > > uses the SPDM over Storage transport as defined by the DMTF > > DSP0286. > > > > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> > > --- > > include/system/spdm-socket.h | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/include/system/spdm-socket.h b/include/system/spdm- > > socket.h > > index 2b7d03f82d..fc007e5b48 100644 > > --- a/include/system/spdm-socket.h > > +++ b/include/system/spdm-socket.h > > @@ -88,6 +88,18 @@ bool spdm_socket_send(const int socket, uint32_t > > socket_cmd, > > */ > > void spdm_socket_close(const int socket, uint32_t transport_type); > > > > +/* > > + * Defines the transport encoding for SPDM, this information shall > > be passed > > + * down to the SPDM server, when conforming to the SPDM over > > Storage standard > > + * as defined by DSP0286. > > + */ > > +typedef struct QEMU_PACKED { > > + uint8_t security_protocol; > > + uint16_t security_protocol_specific; > > + bool inc_512; > > + uint32_t length; > > +} StorageSpdmTransportHeader; > > Does it make sense to pack a bool? Is this defined by the SPDM server > in > use? I can't find the definition of this header anywhere. > This is essentially a virtual header containing essential storage transport data as per DSP0286. For example, this is defined in the upstream effort for `libspmd` to add storage binding support [1] and in DSP0286 [2], this is defined in section 5.1.1. Current implementation of the SPDM server (i.e in `spdm-utils` only one to have support for storage), will just pass this header to `libspdm` to be decoded. Once decoded by `libspdm`, `spdm-utils`/server will contextually check for validity of the message. As for inc_512, it just need to be yes or no, is there a better way to represent that here? [1] https://github.com/DMTF/libspdm/pull/2827/files#diff-30c523edca23983e0f16e067772ec18e711a40f53ac49c8dda24301450b724d0R44 [2] https://www.dmtf.org/sites/default/files/standards/documents/DSP0286_1.0.0WIP90.pdf > > + > > #define SPDM_SOCKET_COMMAND_NORMAL 0x0001 > > #define SPDM_SOCKET_STORAGE_CMD_IF_SEND 0x0002 > > #define SPDM_SOCKET_STORAGE_CMD_IF_RECV 0x0003 > > -- > > 2.47.1 > > > >
On Jan 15 02:16, Wilfred Mallawa wrote: > On Fri, 2025-01-10 at 10:04 +0100, Klaus Jensen wrote: > > On Jan 7 15:29, Wilfred Mallawa via wrote: > > > This header contains the transport encoding for an SPDM message > > > that > > > uses the SPDM over Storage transport as defined by the DMTF > > > DSP0286. > > > > > > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> > > > --- > > > include/system/spdm-socket.h | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/include/system/spdm-socket.h b/include/system/spdm- > > > socket.h > > > index 2b7d03f82d..fc007e5b48 100644 > > > --- a/include/system/spdm-socket.h > > > +++ b/include/system/spdm-socket.h > > > @@ -88,6 +88,18 @@ bool spdm_socket_send(const int socket, uint32_t > > > socket_cmd, > > > */ > > > void spdm_socket_close(const int socket, uint32_t transport_type); > > > > > > +/* > > > + * Defines the transport encoding for SPDM, this information shall > > > be passed > > > + * down to the SPDM server, when conforming to the SPDM over > > > Storage standard > > > + * as defined by DSP0286. > > > + */ > > > +typedef struct QEMU_PACKED { > > > + uint8_t security_protocol; > > > + uint16_t security_protocol_specific; > > > + bool inc_512; > > > + uint32_t length; > > > +} StorageSpdmTransportHeader; > > > > Does it make sense to pack a bool? Is this defined by the SPDM server > > in > > use? I can't find the definition of this header anywhere. > > > This is essentially a virtual header containing essential storage > transport data as per DSP0286. For example, this is defined in the > upstream effort for > `libspmd` to add storage binding support [1] and in DSP0286 [2], this > is defined in section 5.1.1. > > Current implementation of the SPDM server (i.e in `spdm-utils` only one > to have support for storage), will just pass this header to `libspdm` > to be decoded. Once decoded by `libspdm`, `spdm-utils`/server will > contextually check for validity of the message. > OK, understood. > As for inc_512, it just need to be yes or no, is there a better way to > represent that here? It's a byte, right? Then I think using uint8_t is more clear here.
On Tue, 2025-01-28 at 09:03 +0100, Klaus Jensen wrote: > On Jan 15 02:16, Wilfred Mallawa wrote: > > On Fri, 2025-01-10 at 10:04 +0100, Klaus Jensen wrote: > > > On Jan 7 15:29, Wilfred Mallawa via wrote: > > > > This header contains the transport encoding for an SPDM message > > > > that > > > > uses the SPDM over Storage transport as defined by the DMTF > > > > DSP0286. > > > > > > > > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> > > > > --- > > > > include/system/spdm-socket.h | 12 ++++++++++++ > > > > 1 file changed, 12 insertions(+) > > > > > > > > diff --git a/include/system/spdm-socket.h > > > > b/include/system/spdm- > > > > socket.h > > > > index 2b7d03f82d..fc007e5b48 100644 > > > > --- a/include/system/spdm-socket.h > > > > +++ b/include/system/spdm-socket.h > > > > @@ -88,6 +88,18 @@ bool spdm_socket_send(const int socket, > > > > uint32_t > > > > socket_cmd, > > > > */ > > > > void spdm_socket_close(const int socket, uint32_t > > > > transport_type); > > > > > > > > +/* > > > > + * Defines the transport encoding for SPDM, this information > > > > shall > > > > be passed > > > > + * down to the SPDM server, when conforming to the SPDM over > > > > Storage standard > > > > + * as defined by DSP0286. > > > > + */ > > > > +typedef struct QEMU_PACKED { > > > > + uint8_t security_protocol; > > > > + uint16_t security_protocol_specific; > > > > + bool inc_512; > > > > + uint32_t length; > > > > +} StorageSpdmTransportHeader; > > > > > > Does it make sense to pack a bool? Is this defined by the SPDM > > > server > > > in > > > use? I can't find the definition of this header anywhere. > > > > > This is essentially a virtual header containing essential storage > > transport data as per DSP0286. For example, this is defined in the > > upstream effort for > > `libspmd` to add storage binding support [1] and in DSP0286 [2], > > this > > is defined in section 5.1.1. > > > > Current implementation of the SPDM server (i.e in `spdm-utils` only > > one > > to have support for storage), will just pass this header to > > `libspdm` > > to be decoded. Once decoded by `libspdm`, `spdm-utils`/server will > > contextually check for validity of the message. > > > > OK, understood. > > > As for inc_512, it just need to be yes or no, is there a better way > > to > > represent that here? > > It's a byte, right? Then I think using uint8_t is more clear here. As per, SCSI Primary Commands - 6 (SPC-6) it's a single bit to indicate weather it's set or not, hence why I thought a boolean might make more sense here.
diff --git a/include/system/spdm-socket.h b/include/system/spdm-socket.h index 2b7d03f82d..fc007e5b48 100644 --- a/include/system/spdm-socket.h +++ b/include/system/spdm-socket.h @@ -88,6 +88,18 @@ bool spdm_socket_send(const int socket, uint32_t socket_cmd, */ void spdm_socket_close(const int socket, uint32_t transport_type); +/* + * Defines the transport encoding for SPDM, this information shall be passed + * down to the SPDM server, when conforming to the SPDM over Storage standard + * as defined by DSP0286. + */ +typedef struct QEMU_PACKED { + uint8_t security_protocol; + uint16_t security_protocol_specific; + bool inc_512; + uint32_t length; +} StorageSpdmTransportHeader; + #define SPDM_SOCKET_COMMAND_NORMAL 0x0001 #define SPDM_SOCKET_STORAGE_CMD_IF_SEND 0x0002 #define SPDM_SOCKET_STORAGE_CMD_IF_RECV 0x0003
This header contains the transport encoding for an SPDM message that uses the SPDM over Storage transport as defined by the DMTF DSP0286. Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com> --- include/system/spdm-socket.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)