On Mon, Jun 20, 2022 at 10:24:49AM +0100, Jonathan Cameron wrote:
>
> Hi Bjorn,
>
> Thanks for reviewing! Up to Ira of course, but I agree with all your
> comments
Me too! :-D
> - 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.
Yep, CXL just needs to find out which mailbox has CDAT on it.
> >
> > > + */
> > > +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.
I think at this layer the DOE protocol specifies all message sizes are in
multiples of DW's. So I think this layer should enforce that. Other protocols
will need to pad if they need to based on their need.
> Let's avoid this being unclear in future by stating that it's
> in bytes in the comment.
Already done!
Thanks Jonathan!
Ira
>
> 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;
> > > +};
>
