Re: [PATCH V16 3/6] PCI/DOE: Add DOE mailbox support functions

[Dan Williams]

Jonathan Cameron wrote:
> On Mon, 15 Aug 2022 10:17:00 +0800
> "Li, Ming" <ming4.li@xxxxxxxxx> wrote:
> 
> > On 7/20/2022 4:52 AM, ira.weiny@xxxxxxxxx wrote:
> > > From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > > 
> > > Introduced in a PCIe r6.0, sec 6.30, DOE provides a config space based
> > > mailbox with standard protocol discovery.  Each mailbox is accessed
> > > through a DOE Extended Capability.
> > > 
> > > Each DOE mailbox must support the DOE discovery protocol in addition to
> > > any number of additional protocols.
> > > 
> > > Define core PCIe functionality to manage a single PCIe DOE mailbox at a
> > > defined config space offset.  Functionality includes iterating,
> > > creating, query of supported protocol, and task submission.  Destruction
> > > of the mailboxes is device managed.
> > > 
> > > Cc: "Li, Ming" <ming4.li@xxxxxxxxx>
> > > Cc: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> > > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> > > Acked-by: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> > > Co-developed-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> > > Signed-off-by: Ira Weiny <ira.weiny@xxxxxxxxx>
> > > 
> > > ---  
> > 
> > > +/**
> > > + * pci_doe_submit_task() - Submit a task to be processed by the state machine
> > > + *
> > > + * @doe_mb: DOE mailbox capability to submit to
> > > + * @task: task to be queued
> > > + *
> > > + * Submit a DOE task (request/response) to the DOE mailbox to be processed.
> > > + * Returns upon queueing the task object.  If the queue is full this function
> > > + * will sleep until there is room in the queue.
> > > + *
> > > + * task->complete will be called when the state machine is done processing this
> > > + * task.
> > > + *
> > > + * Excess data will be discarded.
> > > + *
> > > + * RETURNS: 0 when task has been successfully queued, -ERRNO on error
> > > + */
> > > +int pci_doe_submit_task(struct pci_doe_mb *doe_mb, struct pci_doe_task *task)
> > > +{
> > > +	if (!pci_doe_supports_prot(doe_mb, task->prot.vid, task->prot.type))
> > > +		return -EINVAL;
> > > +
> > > +	/*
> > > +	 * DOE requests must be a whole number of DW and the response needs to
> > > +	 * be big enough for at least 1 DW
> > > +	 */
> > > +	if (task->request_pl_sz % sizeof(u32) ||
> > > +	    task->response_pl_sz < sizeof(u32))
> > > +		return -EINVAL;
> > > +
> > > +	if (test_bit(PCI_DOE_FLAG_DEAD, &doe_mb->flags))
> > > +		return -EIO;
> > > +
> > > +	task->doe_mb = doe_mb;
> > > +	INIT_WORK(&task->work, doe_statemachine_work);
> > > +	queue_work(doe_mb->work_queue, &task->work);
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_doe_submit_task);  
> > 
> 
> > I'm curious why there is not a sychronous function for DOE users. I
> > think some protocols over DOE need a DOE sychrounous interface(e.g.
> > SPDM, TDISP, IDE...). These protocols have to implement an sychronous
> > function over pci_doe_submit_task() by themselves, maybe we can
> > implement an generic sychronous function in PCI/DOE for these DOE
> > users.
> 
> This came up during discussions of earlier versions of the series.
> IIRC we decided to postpone adding a synchronous wrapper function
> until there were more use cases.

Note that as far as I can see TDISP makes no requirement of the VMM nor
TVM (trusted VM) to drive an SPDM session. All of that is handled by the
TSM (TEE / Platform Security Manager) where the VMM is outside the trust
boundary. The TVM only validates the contents of a
GET_DEVICE_INTERFACE_REPORT which the TSM had negotiated with the DSM
(Device Security Manager).

All this to say its not clear that Linux needs to grow the capabilities
listed above when it is expected to be outside the Trusted Compute
Boundary.