Hi Bjorn, Thanks for reviewing! Up to Ira of course, but I agree with all your comments - a few responses to questions follow. > > > + * pci_doe_supports_prot() - Return if the DOE instance supports the given > > + * protocol > > + * @doe_mb: DOE mailbox capability to query > > + * @vid: Protocol Vendor ID > > + * @type: Protocol type > > + * > > + * RETURNS: True if the DOE mailbox supports the protocol specified > > Is the typical use that the caller has a few specific protocols it > cares about? There's no case where a caller might want to enumerate > them all? I guess they're all in prots[], but that's supposed to be > opaque to users. Given each protocol needs specific handling in the driver, the only usecase for a general enumeration would be debug I think. Maybe it makes sense to provide that info to userspace somewhere, but definitely feels like something for a follow up discussion. > > > + */ > > +bool pci_doe_supports_prot(struct pci_doe_mb *doe_mb, u16 vid, u8 type) > > +{ > > + int i; > > + > > + /* The discovery protocol must always be supported */ > > + if (vid == PCI_VENDOR_ID_PCI_SIG && type == PCI_DOE_PROTOCOL_DISCOVERY) > > + return true; > > + > > + for (i = 0; i < doe_mb->num_prots; i++) > > + if ((doe_mb->prots[i].vid == vid) && > > + (doe_mb->prots[i].type == type)) > > + return true; > > + > > + return false; > > +} > > +EXPORT_SYMBOL_GPL(pci_doe_supports_prot); > > > + * struct pci_doe_task - represents a single query/response > > + * > > + * @prot: DOE Protocol > > + * @request_pl: The request payload > > + * @request_pl_sz: Size of the request payload > > Size is in dwords, not bytes, I guess? It's in bytes (IIRC) - we divide it by. It's a bit of a mess, but there are parts of SPDM over CMA where messages are not full number of dwords. My thinking was that we 'might' move the padding into the generic code if this becomes something multiple protocols need. For now the RFC does the padding at the CMA layer. Let's avoid this being unclear in future by stating that it's in bytes in the comment. Jonathan > > > + * @response_pl: The response payload > > + * @response_pl_sz: Size of the response payload > > + * @rv: Return value. Length of received response or error > > + * @complete: Called when task is complete > > + * @private: Private data for the consumer > > + */ > > +struct pci_doe_task { > > + struct pci_doe_protocol prot; > > + u32 *request_pl; > > + size_t request_pl_sz; > > + u32 *response_pl; > > + size_t response_pl_sz; > > + int rv; > > + void (*complete)(struct pci_doe_task *task); > > + void *private; > > +};