Re: [RFC PATCH v2 1/2] Basic PCIe DOE support

[Chris Browy]

> On Feb 18, 2021, at 2:11 PM, Jonathan Cameron <jonathan.cameron@huawei.com> 
> wrote:
> 
> On Fri, 12 Feb 2021 16:58:21 -0500
> Chris Browy <cbrowy@avery-design.com> wrote:
> 
>>> On Feb 12, 2021, at 11:24 AM, Jonathan Cameron 
>>> <Jonathan.Cameron@Huawei.com> wrote:
>>> 
>>> On Tue, 9 Feb 2021 15:35:49 -0500
>>> Chris Browy <cbrowy@avery-design.com> wrote:
>>> 
>>> Run ./scripts/checkpatch.pl over the patches and fix all the warnings before
>>> posting.  It will save time by clearing out most of the minor formatting 
>>> issues
>>> and similar that inevitably sneak in during development.
>>> 
>> Excellent suggestion.  We’re still newbies!
>> 
>>> The biggest issue I'm seeing in here is that the abstraction of
>>> multiple DOE capabiltiies accessing same protocols doesn't make sense.
>>> 
>>> Each DOE ecap region and hence mailbox can have it's own set of
>>> (possibly  overlapping) protocols.
>>> 
>>> From the ECN:
>>> "It is permitted for a protocol using data object exchanges to require
>>> that a Function implement a unique instance of DOE for that specific
>>> protocol, and/or to allow sharing of a DOE instance to only a specific
>>> set of protocols using data object exchange, and/or to allow a Function
>>> to implement multiple instances of DOE supporting the specific protocol."
>>> 
>>> Tightly couple the ECAP and DOE.  If we are in the multiple instances
>>> of DOE supporting a specific protocol case, then register it separately
>>> for each one.  The individual device emulation then needs to deal with
>>> any possible clashes etc.  
>> 
>> Not sure how configurable we want to make the device.  It is a simple type 3
>> device after all. 
> 
> Agreed, but what I (or someone else) really doesn't want to have to do
> in the future is reimplement DOE because we made design decisions that make
> this version hard to reuse.  Unless it is particularly nasty to do we should
> try to design something that is generally useful rather than targeted to
> closely at the specific case we are dealing with.
> 
> I'd argue the ECAP and the DOE mailbox are always tightly coupled 1-to-1.
> Whether the device wants to implement multiple protocols on each DOE mailbox
> or indeed run individual protocols on multiple DOE mailboxes is a design
> decision, but the actual mechanics of DOE match up with the config
> space structures anything else is impdef on the device.

Yes I agree that there is 1-to-1 between DOE extended cap (ECAP) and DOE
Mailbox.  If we want to provide complete flexibility we should let the user 
pass 
device property arrays to QEMU command for how many DOE ECAP’s to build 
out and how to assign protocol(s) to each of them.  Array index is the DOE 
instance #.

Also we can provide a property for cdat binary (blob) filename to initialize 
the CDAT structure[entries].  This just reads in whatever mix of CDAT structure
types are in the blob.

-device cxl-type3,bus=rp0,memdev=cxl-mem1,id=cxl-pmem0,size=256M \
    doe-ecap-instances=2 \
    doe-ecap[0]=5 // bitwise OR for protocols shared
    doe-ecap[1]=2 //bitwise OR for protocols shared
    doe-ecap-cdat[1]=mycdat.bin

where let’s say protocols bitvector
bit [0]=CMA
bit [1]=CDAT
bit [2}=Compliance

Let me know if you some better alternatives and we’ll implement it.


> 
>> 
>> The DOE spec does leave it pretty arbitrary regarding N DOE instances (DOE 
>> Extended Cap entry points) for M protocols, including where N>1 and M=1.  
>> Currently we implement N=2 DOE caps (instances), one for CDAT, one for 
>> Compliance Mode.[
>> 
>> Maybe a more complex MLD device might have one or more DOE instances 
>> for the CDAT protocol alone to define each HDM but currently we only have 
>> one pmem (SLD) so we can’t really do much more than what’s supported.
>> 
>> Open to further suggestion though.  Based on answer to above we’ll follow 
>> the suggestion lower in the code review regarding 
>> 
> ...
>