Re: [PATCH RFC 0/5] hw/cxl: Type 2 Device RFC

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Fri, 18 Oct 2024 16:25:58 +0100
Alejandro Lucero Palau <alucerop@amd.com> wrote:

> On 10/18/24 15:49, Zhi Wang wrote:
> > On 17/10/2024 19.57, Cédric Le Goater wrote:  
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> Hello,
> >>
> >> On 5/18/23 04:45, Ira Weiny wrote:  
> >>> Type 2 devices are not yet a reality.  Developing core kernel support
> >>> is difficult without some test device to model against.
> >>>
> >>> Define a type 2 device 'cxl-accel'.  This device is derived from the
> >>> type 3 device and retains all that functionality for now.
> >>>
> >>> Mock up a couple of accelerator features (Back Invalidate [BI] and
> >>> Unordered IO [UIO]) as examples for the RFC.  These have no
> >>> functionality other than to report the features as present for software
> >>> to key off of.
> >>>
> >>> Defining these devices in qemu can be done with the following example:
> >>>
> >>> ...
> >>>     -device
> >>> cxl-accel,bus=sw0p0,volatile-memdev=cxl-ac-mem5,id=cxl-dev5,sn=0xCAFE0005
> >>> ...
> >>>
> >>> NOTE: I'm leaving off Michael Tsirkin for now because this is really
> >>> rough and I'm mainly sending it out because it was talked about in the
> >>> CXL community call on 5/16.
> >>>
> >>> Not-Yet-Signed-off-by: Ira Weiny <ira.weiny@intel.com>  
> >>
> >> A recent proposal to add support in VFIO for CXL passthrough uses
> >> this emulated device and a sample driver for a proof of concept.
> >> For this effort, it would be very useful to have a QEMU model for
> >> a CXL type2 device, even partially implemented.
> >>
> >> I haven't found any updates of this series. What are the plans for
> >> upstream today ?
> >>  
> > I was discussing this with Ira and Jonathan in the LPC and CXL discord
> > groups. (Irq and Jonathan, please feel free to correct me) I think we
> > all thought that having the emulated device support in QEMU and a sample
> > CXL type-2 driver in the upstream kernel could be valuable for
> >
> > 1) people who wish to contribute and propose changes, like refactor, and
> > code tweaks related to CXL type-2 support in the kernel. They can
> > validate their patches to check if they break something without a CXL
> > type-2 device.
> >
> > 2) people who wish to contribute on solving problems of CXL type-2
> > generic code, e.g. loading sequences of modules... They can get involved
> > without a real HW.
> >
> > My gut feeling is I can start to work with folks to get it into the
> > mainline together with the sample driver when CXL type-2 support is
> > merged. So people can play with it.  
> 
> 
> I did use an emulated Type2 device for my initial development and I'm 
> still using it for wider testing. It is basically same than the Type3 
> with small changes. I think a proper solution should include command 
> line options for configuring the device with flexibility or maybe 
> referring to a file with that specific configuration. Note there exist 
> optional functionalities like HDM decoder, and CXL.cache will need such 
> a flexibility as well.

In the first instance, I'd just make up a single reasonable configuration.
Later, it may make sense to make it flexible.


> 
> The RFC contains the driver for such emulated device implemented inside 
> the tools/testing/cxl directory, although it has changed since then, but 
> happy to share it. It is almost equal to the code inside efx_cxl.c along 
> with the functions/macros for defining the driver.
> 
> FWIW, although I'm working on Type2 support, I really think qemu could 
> help for development and testing complex CXL infrastructures, more for 
> memory expanders aka Type3 than Type2. I know this requires a good 
> effort but IMO, it will pay off.

I'm definitely not against adding support and Zhi's rough sketch sounds
about right.

> 
> > 3) people who would like to try on vfio-cxl.
> >
> > What would be nice to include in the patchset V1 in my mind:
> >
> > (Ira and Jonathan and folks please feel free to comment)
> >
> > Must have:
> >    - HDM decoder support (>1 HDM decoder). (which I have done it in my
> > tree, so the CXL core can create a CXL region)
> >
> > Nice to have:
> >    - CXLType2 device system reset handler. As what mentioned in my cover
> > letter, a system reset handler is required to reset the device state.
> > Linux kernel CXL core assumes the HW is pre-configured by FW if it sees
> > CXL.mem is enabled when enumerating the device. So I have to kill QEMU
> > instead of resetting the virtual machine when debugging.
> >
> >    - CXLType2 device in the patch should be derived from PCIDevice
> > (current it is derived from the CXLType3 device, which carries quite
> > some unnecessary stuff to present to the guest)
> >
> >    - Refactor of the current QEMU FWMS code to deliver the guest access
> > to the virtual memory backend to the correct emulated CXL device (which
> > is currently hardcoded to connect to cxl-type3 device in QEMU. Without
> > this, the access to the CXL region from the guest seems problematic but
> > creating CXL region is still fine.)
> >
> > Thanks,
> > Zhi.
> >  
> >> For vfio-cxl, see :
> >>
> >> * [RFC] vfio: introduce vfio-cxl to support CXL type-2 accelerator
> >> passthrough
> >>     https://lore.kernel.org/kvm/20240920223446.1908673-1-zhiw@nvidia.com
> >> * [RFC] Introduce vfio-cxl to support CXL type-2 device passthrough
> >>     https://lore.kernel.org/all/20240921071440.1915876-1-zhiw@nvidia.com/
> >>
> >>
> >> Thanks,
> >>
> >> C.
> >>
> >>
> >>  
> >>> ---
> >>> Ira Weiny (5):
> >>>         hw/cxl: Use define for build bug detection
> >>>         hw/cxl: Refactor component register initialization
> >>>         hw/cxl: Derive a CXL accelerator device from Type-3
> >>>         hw/cxl/accel: Add Back-Invalidate decoder capbility structure
> >>>         hw/cxl: Add UIO HDM decoder register fields
> >>>
> >>>    docs/system/devices/cxl.rst    | 11 ++++++
> >>>    hw/cxl/cxl-component-utils.c   | 80
> >>> +++++++++++++++++++-----------------------
> >>>    hw/mem/cxl_type3.c             | 39 ++++++++++++++++++++
> >>>    include/hw/cxl/cxl_component.h | 51 +++++++++++++++++++--------
> >>>    include/hw/cxl/cxl_device.h    | 16 +++++++++
> >>>    include/hw/pci/pci_ids.h       |  1 +
> >>>    6 files changed, 141 insertions(+), 57 deletions(-)
> >>> ---
> >>> base-commit: 8eb2a03258313f404ca0c8609a8f9009b9b4318c
> >>> change-id: 20230517-rfc-type2-dev-c2d661a29d96
> >>>
> >>> Best regards,