Re: [PATCH v4 2/5] PCI/DOE: Add Data Object Exchange support

On Thu, 8 Jul 2021 09:48:15 +0100
Lorenzo Pieralisi [off-list ref] wrote:

On Wed, Jul 07, 2021 at 12:54:33PM -0700, Dan Williams wrote:

quoted

On Thu, Jun 10, 2021 at 1:06 PM Dan Williams [off-list ref] wrote:  

quoted

On Mon, May 24, 2021 at 6:41 AM Jonathan Cameron
[off-list ref] wrote:  

quoted

Introduced in a PCI ECN [1], DOE provides a config space based mailbox with
standard protocol discovery.  Each mailbox is accessed through a DOE
Extended Capability.

A device may have 1 or more DOE mailboxes, each of which is allowed to
support any number of protocols (some DOE protocol specifications apply
additional restrictions).  A given protocol may be supported on more than
one DOE mailbox on a given function.

If a driver wishes to access any number of DOE instances / protocols it
makes a single call to pci_doe_register_all() which will find available
DOEs, create the required infrastructure and cache the protocols they
support.  pci_doe_find() can then retrieve a pointer to an appropriate DOE
instance.

A synchronous interface is provided in pci_doe_exchange_sync() to perform a
single query / response exchange.

Testing conducted against QEMU using:

https://lore.kernel.org/qemu-devel/1619454964-10190-1-git-send-email-cbrowy@avery-design.com/ (local)  

Nice.

I was hoping that by now QEMU upstream would have given us some
indication that this useful work that has a chance of being merged. I
fear it's only us CXL practitioner's that care. Perhaps the PCI IDE
support will get them to move on at least the DOE patches?
 

quoted

[1] https://members.pcisig.com/wg/PCI-SIG/document/14143
    Data Object Exchange (DOE) - Approved 12 March 2020

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  

The core logic of this looks good to me. The interfaces for other code
to make use of this I feel can lean heavier on existing mechanics. A
few points come to mind:

- Does this need to support anything more than queue depth 1? I know
the specification seems to allow for some overlapping and queueing,
but I don't think there are any use cases that are precluded if the
max number of tasks in flight for a given DOE is one.

We aren't handling overlapping today, so a queue of one and wait on access
to that would work fine.  Queuing is more likely to be about multiple protocols
sharing a DOE (where allowed) as I'm not seeing a need in any individual protocol
to queue lots of things up at once. You could queue a few things in CMA as they
aren't all dependent on each other (though the order matters for building the
measurements) but I'm not sure why you would.

quoted

quoted

- Once its queue depth 1 then the list of tasks can be replaced with a
wait_queue_head_t where submitters wait for the previous task to
finish.

I'll go with a resounding 'maybe' on that... I'm not sure I like that the
async path in the submitter could be waiting a very long time (if we think
anyone will actually do async...)  I'm probably missing something.

quoted

quoted

- This appears to be the prototypical scenario for deploying the new
auxiliary bus facility. Rather than custom code device-like facilities
(lists and parents etc) in 'struct pci_doe' just make pci_doe a device
directly (auxiliary-device) and separate the infrastructure that
drives that device to a driver (auxiliary-driver). That makes the
lifetime management more idiomatic, allows for user space to have
typical driver-binding controls to manage kernel-user DOE conflicts,
and it allows for typical driver services like devm.  

Hi Jonathan,

Are you waiting on me to take a shot at refactoring the DOE driver
into this proposed auxiliary device/driver organization? I am happy to
do that if you've moved on to looking at the kernel-side SPDM
implementation [1].

I wasn't really expecting you to take a shot, but if you have time, that
would be great. I've not looked at the auxiliary bus stuff yet.

Whilst I have a PoC up and running for SPDM, it's a bunch of dirty hacks and
I suspect that, even when cleaned up, it's going to keep me busy for a while.

Mostly I've developed a steadily growing hatred for the different ways in
which signatures can be stored and hand decoding records or digging in
openspdm to figure out what order and encoding things are in this time...

quoted

I would expect DOE,  SPDM, and IDE would be a useful topic to discuss
at the the Plumbers PCI Microconference assuming we do not solve all
the open issues before September.  

Definitely, I will make sure we schedule a slot on these topics.

+1

I'm sure there will be some issues left :)

Thanks,
Lorenzo

quoted

[1]: https://lore.kernel.org/r/20210629132520.00000d1f@Huawei.com (local)