On Tue, Mar 3, 2026 at 10:09 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Wed, 11 Feb 2026 13:29:20 +1000
> alistair23@gmail.com wrote:
>
> > From: Alistair Francis <alistair@alistair23.me>
> >
> > Support the GET_CAPABILITIES SPDM command.
> >
> > Signed-off-by: Alistair Francis <alistair@alistair23.me>
> A few comments inline.
> Looks in pretty good shape to me but definitely needs review
> by those more familiar with rust than me!
>
> > ---
> >  lib/rspdm/consts.rs    | 18 ++++++++--
> >  lib/rspdm/lib.rs       |  4 +++
> >  lib/rspdm/state.rs     | 68 +++++++++++++++++++++++++++++++++++--
> >  lib/rspdm/validator.rs | 76 +++++++++++++++++++++++++++++++++++++++++-
> >  4 files changed, 161 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/rspdm/consts.rs b/lib/rspdm/consts.rs
> > index 38f48f0863e2..a6a66e2af1b5 100644
> > --- a/lib/rspdm/consts.rs
> > +++ b/lib/rspdm/consts.rs
> > @@ -12,9 +12,7 @@
> >
> >  /* SPDM versions supported by this implementation */
> >  pub(crate) const SPDM_VER_10: u8 = 0x10;
> > -#[allow(dead_code)]
> >  pub(crate) const SPDM_VER_11: u8 = 0x11;
> > -#[allow(dead_code)]
> >  pub(crate) const SPDM_VER_12: u8 = 0x12;
> >  pub(crate) const SPDM_VER_13: u8 = 0x13;
> >
> > @@ -132,3 +130,19 @@ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> >
> >  pub(crate) const SPDM_GET_VERSION: u8 = 0x84;
> >  pub(crate) const SPDM_GET_VERSION_LEN: usize = mem::size_of::<SpdmHeader>() + 255;
> > +
> > +pub(crate) const SPDM_GET_CAPABILITIES: u8 = 0xe1;
> > +pub(crate) const SPDM_MIN_DATA_TRANSFER_SIZE: u32 = 42;
> Spec reference?  C code refers to /* SPDM 1.2.0 margin no 226 */
> It's 269 in 1.3.1
>
> > +
> > +// SPDM cryptographic timeout of this implementation:
> > +// Assume calculations may take up to 1 sec on a busy machine, which equals
> > +// roughly 1 << 20.  That's within the limits mandated for responders by CMA
> > +// (1 << 23 usec, PCIe r6.2 sec 6.31.3) and DOE (1 sec, PCIe r6.2 sec 6.30.2).
> > +// Used in GET_CAPABILITIES exchange.
> > +pub(crate) const SPDM_CTEXPONENT: u8 = 20;
> > +
> > +pub(crate) const SPDM_CERT_CAP: u32 = 1 << 1;
>
> Can we use bit_u32() or similar here?

Yep, fixed

> We've tried to move to BIT() for similar defines in the C code, so I'm
> looking for something along those lines. Provides a form of documentation
> that they are single bit fields.
>
>
> > +pub(crate) const SPDM_CHAL_CAP: u32 = 1 << 2;
> > +
> > +pub(crate) const SPDM_REQ_CAPS: u32 = SPDM_CERT_CAP | SPDM_CHAL_CAP;
> > +pub(crate) const SPDM_RSP_MIN_CAPS: u32 = SPDM_CERT_CAP | SPDM_CHAL_CAP;
> > diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs
> > index 08abcbb21247..5f6653ada59d 100644
> > --- a/lib/rspdm/lib.rs
> > +++ b/lib/rspdm/lib.rs
> > @@ -113,6 +113,10 @@
> >          return e.to_errno() as c_int;
> >      }
> >
> > +    if let Err(e) = state.get_capabilities() {
> > +        return e.to_errno() as c_int;
> > +    }
> > +
> >      0
> >  }
> >
> > diff --git a/lib/rspdm/state.rs b/lib/rspdm/state.rs
> > index 3ad53ec05044..0efad7f341cd 100644
> > --- a/lib/rspdm/state.rs
> > +++ b/lib/rspdm/state.rs
> > @@ -17,9 +17,12 @@
> >  };
> >
> >  use crate::consts::{
> > -    SpdmErrorCode, SPDM_ERROR, SPDM_GET_VERSION_LEN, SPDM_MAX_VER, SPDM_MIN_VER, SPDM_REQ,
> > +    SpdmErrorCode, SPDM_ERROR, SPDM_GET_VERSION_LEN, SPDM_MAX_VER, SPDM_MIN_DATA_TRANSFER_SIZE,
> > +    SPDM_MIN_VER, SPDM_REQ, SPDM_RSP_MIN_CAPS, SPDM_VER_10, SPDM_VER_11, SPDM_VER_12,
> > +};
>
> This is why I was getting bit grumpy in previous about long lines.
> Fair enough if this is only option, but it's not nice for a review process
> that relies on patches!

Also fixed

>
>
> > +use crate::validator::{
> > +    GetCapabilitiesReq, GetCapabilitiesRsp, GetVersionReq, GetVersionRsp, SpdmErrorRsp, SpdmHeader,
> >  };
> > -use crate::validator::{GetVersionReq, GetVersionRsp, SpdmErrorRsp, SpdmHeader};
> >
> >  /// The current SPDM session state for a device. Based on the
> >  /// C `struct spdm_state`.
> > @@ -35,6 +38,8 @@
> >  ///
> >  /// `version`: Maximum common supported version of requester and responder.
> >  ///  Negotiated during GET_VERSION exchange.
> > +/// @rsp_caps: Cached capabilities of responder.
> > +///  Received during GET_CAPABILITIES exchange.
> >  #[expect(dead_code)]
> >  pub struct SpdmState {
> >      pub(crate) dev: *mut bindings::device,
> > @@ -46,6 +51,7 @@ pub struct SpdmState {
> >
> >      // Negotiated state
> >      pub(crate) version: u8,
> > +    pub(crate) rsp_caps: u32,
> >  }
> >
> >  impl SpdmState {
> > @@ -65,6 +71,7 @@ pub(crate) fn new(
> >              keyring,
> >              validate,
> >              version: SPDM_MIN_VER,
> > +            rsp_caps: 0,
> >          }
> >      }
> >
> > @@ -269,4 +276,61 @@ pub(crate) fn get_version(&mut self) -> Result<(), Error> {
> >
> >          Ok(())
> >      }
> > +
> > +    /// Obtain the supported capabilities from an SPDM session and store the
> > +    /// information in the `SpdmState`.
> > +    pub(crate) fn get_capabilities(&mut self) -> Result<(), Error> {
> > +        let mut request = GetCapabilitiesReq::default();
> > +        request.version = self.version;
> > +
> > +        let (req_sz, rsp_sz) = match self.version {
> > +            SPDM_VER_10 => (4, 8),
> > +            SPDM_VER_11 => (8, 8),
> > +            _ => {
> > +                request.data_transfer_size = self.transport_sz.to_le();
> > +                request.max_spdm_msg_size = request.data_transfer_size;
> > +
> > +                (
> > +                    core::mem::size_of::<GetCapabilitiesReq>(),
> > +                    core::mem::size_of::<GetCapabilitiesRsp>(),
> > +                )
> > +            }
> > +        };
> > +
> > +        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
> > +        let request_buf = unsafe { from_raw_parts_mut(&mut request as *mut _ as *mut u8, req_sz) };
> > +
> > +        let mut response_vec: KVec<u8> = KVec::with_capacity(rsp_sz, GFP_KERNEL)?;
> > +        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
> > +        let response_buf = unsafe { from_raw_parts_mut(response_vec.as_mut_ptr(), rsp_sz) };
> > +
> > +        let rc = self.spdm_exchange(request_buf, response_buf)?;
> > +
> > +        if rc < (rsp_sz as i32) {
> > +            pr_err!("Truncated capabilities response\n");
> > +            to_result(-(bindings::EIO as i32))?;
> > +        }
> > +
> > +        // SAFETY: `rc` is the length of data read, which will be smaller
> > +        // then the capacity of the vector
>
> Fairly sure it's exactly the size of the of vector in this case.
> Bit (0) of param1 isn't set sop the algorithms block is never included.

It is, my point was that it won't overflow. I'll adjust this to be more clear

>
> > +        unsafe { response_vec.inc_len(rc as usize) };
> > +
> > +        let response: &mut GetCapabilitiesRsp =
> > +            Untrusted::new_mut(&mut response_vec).validate_mut()?;
> > +
> > +        self.rsp_caps = u32::from_le(response.flags);
>
> Isn't response packed? So why don't we need the read_unaligned stuff in this
> case but did in the previous patch?

We need it in the previous patch as we are using a pointer to iterate
over the dynamic length version_number_entries array, that doesn't
apply here

>
> > +        if (self.rsp_caps & SPDM_RSP_MIN_CAPS) != SPDM_RSP_MIN_CAPS {
> > +            to_result(-(bindings::EPROTONOSUPPORT as i32))?;
> > +        }
> > +
> > +        if self.version >= SPDM_VER_12 {
> > +            if response.data_transfer_size < SPDM_MIN_DATA_TRANSFER_SIZE {
> > +                pr_err!("Malformed capabilities response\n");
> > +                to_result(-(bindings::EPROTO as i32))?;
> > +            }
> > +            self.transport_sz = self.transport_sz.min(response.data_transfer_size);
> > +        }
> > +
> > +        Ok(())
> > +    }
> >  }
> > diff --git a/lib/rspdm/validator.rs b/lib/rspdm/validator.rs
> > index f69be6aa6280..cd792c46767a 100644
> > --- a/lib/rspdm/validator.rs
> > +++ b/lib/rspdm/validator.rs
> > @@ -16,7 +16,7 @@
> >      validate::{Unvalidated, Validate},
> >  };
> >
> > -use crate::consts::SPDM_GET_VERSION;
> > +use crate::consts::{SPDM_CTEXPONENT, SPDM_GET_CAPABILITIES, SPDM_GET_VERSION, SPDM_REQ_CAPS};
> >
> >  #[repr(C, packed)]
> >  pub(crate) struct SpdmHeader {
> > @@ -131,3 +131,77 @@ fn validate(unvalidated: &mut Unvalidated<KVec<u8>>) -> Result<Self, Self::Err>
> >          Ok(rsp)
> >      }
> >  }
> > +
> > +#[repr(C, packed)]
> > +pub(crate) struct GetCapabilitiesReq {
> > +    pub(crate) version: u8,
> > +    pub(crate) code: u8,
> > +    pub(crate) param1: u8,
> > +    pub(crate) param2: u8,
> > +
> > +    reserved1: u8,
> > +    pub(crate) ctexponent: u8,
> > +    reserved2: u16,
> > +
> > +    pub(crate) flags: u32,
> > +
> > +    /* End of SPDM 1.1 structure */
> > +    pub(crate) data_transfer_size: u32,
> > +    pub(crate) max_spdm_msg_size: u32,
> > +}
> > +
> > +impl Default for GetCapabilitiesReq {
> > +    fn default() -> Self {
> > +        GetCapabilitiesReq {
> > +            version: 0,
> > +            code: SPDM_GET_CAPABILITIES,
> > +            param1: 0,
>
> Maybe add some comment on impacts of bit 0.
>
> > +            param2: 0,
> > +            reserved1: 0,
> > +            ctexponent: SPDM_CTEXPONENT,
> > +            reserved2: 0,
> > +            flags: (SPDM_REQ_CAPS as u32).to_le(),
> > +            data_transfer_size: 0,
> > +            max_spdm_msg_size: 0,
> > +        }
> > +    }
> > +}
> > +
> > +#[repr(C, packed)]
> > +pub(crate) struct GetCapabilitiesRsp {
> > +    pub(crate) version: u8,
> > +    pub(crate) code: u8,
> > +    pub(crate) param1: u8,
> > +    pub(crate) param2: u8,
>
> param 2 is reserved in 1.3.1  Maybe not pub(crate)?
> If it is not reserved in some particular version perhaps add a comment.
>
> > +
> > +    reserved1: u8,
> > +    pub(crate) ctexponent: u8,
> > +    reserved2: u16,
> > +
> > +    pub(crate) flags: u32,
> > +
> > +    /* End of SPDM 1.1 structure */
> > +    pub(crate) data_transfer_size: u32,
> > +    pub(crate) max_spdm_msg_size: u32,
> > +
> > +    pub(crate) supported_algorithms: __IncompleteArrayField<__le16>,
> > +}
> > +
> > +impl Validate<&mut Unvalidated<KVec<u8>>> for &mut GetCapabilitiesRsp {
> > +    type Err = Error;
> > +
> > +    fn validate(unvalidated: &mut Unvalidated<KVec<u8>>) -> Result<Self, Self::Err> {
> > +        let raw = unvalidated.raw_mut();
> > +        if raw.len() < mem::size_of::<GetCapabilitiesRsp>() {
>
> So we fail if 1.1?  If that's the case I think we should fail when doing
> the version establishment in the previous patch.

Good catch, we shouldn't fail. I'll fix this

Alistair

>
> The c code had some size mangling to deal with this difference.
>
> > +            return Err(EINVAL);
> > +        }
> > +
> > +        let ptr = raw.as_mut_ptr();
> > +        // CAST: `GetCapabilitiesRsp` only contains integers and has `repr(C)`.
> > +        let ptr = ptr.cast::<GetCapabilitiesRsp>();
> > +        // SAFETY: `ptr` came from a reference and the cast above is valid.
> > +        let rsp: &mut GetCapabilitiesRsp = unsafe { &mut *ptr };
> > +
> > +        Ok(rsp)
> > +    }
> > +}
>