Re: [PATCH 1/4] hw/pcie: Support enabling flit mode

[Markus Armbruster <armbru@redhat.com>]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Aug 11, 2025 at 04:57:41PM +0100, Jonathan Cameron wrote:
>> On Sun, 10 Aug 2025 20:34:02 -0700
>> Davidlohr Bueso <dave@stgolabs.net> wrote:
>> 
>> > As with the link speed and width training, have ad-hoc property for
>> > setting the flit mode and allow CXL components to make use of it.

I understand this is property "256b-flit".

> I note we didn't make it a stable API for width, want to
> prefix this one with "x-", too?

The only use of "256b-flit" is in docs/system/devices/cxl.rst [PATCH
4/4]:

    diff --git a/docs/system/devices/cxl.rst b/docs/system/devices/cxl.rst
    index bf7908429af8..4815de0f2dc4 100644
    --- a/docs/system/devices/cxl.rst
    +++ b/docs/system/devices/cxl.rst
    @@ -384,6 +384,29 @@ An example of 4 devices below a switch suitable for 1, 2 or 4 way interleave::
       -device cxl-type3,bus=swport3,persistent-memdev=cxl-mem3,lsa=cxl-lsa3,id=cxl-pmem3,sn=0x4 \
       -M cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=4k

    +An example of 4 type3 devices with volatile memory below a switch. Two of the devices
    +use HDM-DB for coherence::
    +
    +  qemu-system-x86_64 -M q35,cxl=on -m 4G,maxmem=8G,slots=8 -smp 4 \
    +  ...
    +  -object memory-backend-file,id=cxl-mem0,share=on,mem-path=/tmp/cxltest.raw,size=256M \
    +  -object memory-backend-file,id=cxl-mem1,share=on,mem-path=/tmp/cxltest1.raw,size=256M \
    +  -object memory-backend-file,id=cxl-mem2,share=on,mem-path=/tmp/cxltest2.raw,size=256M \
    +  -object memory-backend-file,id=cxl-mem3,share=on,mem-path=/tmp/cxltest3.raw,size=256M \
    +  -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \
    +  -device cxl-rp,port=0,bus=cxl.1,id=root_port0,chassis=0,slot=0 \
    +  -device cxl-rp,port=1,bus=cxl.1,id=root_port1,chassis=0,slot=1 \
--> +  -device cxl-upstream,bus=root_port0,id=us0,256b-flit=on \
    +  -device cxl-downstream,port=0,bus=us0,id=swport0,chassis=0,slot=4 \
--> +  -device cxl-type3,bus=swport0,volatile-memdev=cxl-mem0,id=cxl-mem0,sn=0x1,256b-flit=on,hdm-db=on \
    +  -device cxl-downstream,port=1,bus=us0,id=swport1,chassis=0,slot=5 \
--> +  -device cxl-type3,bus=swport1,volatile-memdev=cxl-mem1,id=cxl-mem1,sn=0x2,256b-flit=on,hdm-db=on \
    +  -device cxl-downstream,port=2,bus=us0,id=swport2,chassis=0,slot=6 \
    +  -device cxl-type3,bus=swport2,volatile-memdev=cxl-mem2,id=cxl-mem2,sn=0x3 \
    +  -device cxl-downstream,port=3,bus=us0,id=swport3,chassis=0,slot=7 \
    +  -device cxl-type3,bus=swport3,volatile-memdev=cxl-mem3,id=cxl-mem3,sn=0x4 \
    +  -M cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=4k
    +

The purpose of 256b-flit=on is not explained there.  The only hint I can
find is in the commit message: "Devices require enabling Flit mode."
Which devices require what and when is less than clear.  Maybe hdm-db=on
requires 256b-flit=on?

Why does device cxl-upstream use 256b-flit=on without hdm-db=on?

I can't see where property "hdm-db" is defined.  Davidlohr, can you show
me?

I doubt users can figure out how to use this unless they're intimately
familiar with... what exactly?

>> > For the CXL root port and dsp cases, always report flit mode but
>> > the actual value after 'training' will depend on the downstream
>> > device configuration.
>> > 
>> > Suggested-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
>> 
>> Definitely include PCI maintainers on this.
>> +CC Michael, and Marcel.
>> 
>> Also Markus for command line bits.
>> 
>> Looks good to me.
>> 
>> Jonathan

[...]