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

[Davidlohr Bueso]

On Tue, 09 Sep 2025, Markus Armbruster wrote:

"Michael S. Tsirkin" <m...@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 <d...@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?

Yes, hdm-db=on requires 256b-flit=on.

+    if (!ct3d->flitmode && ct3d->hdmdb) {
+        error_setg(errp, "hdm-db requires operating in 256b flit");
+        return false;
+    }

The reason why it's not documented in 'cxl.rst' is because it's out of scope 
really,
and anyone who wants to go out of their way and use a CXL feature that requires 
256B
flit will certainly know about it (or find out with the error above).

I'll go ahead and add some text in the example description, but the example 
itself
is pretty clear imo. I will also add some relevant text in the changelog of 
patch 1/4.

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?

Yes, that was a slip up while rebasing, I caught that missing while preparing 
v3.

Thanks for the feedback.