Re: [PATCH v8 01/46] hw/pci/cxl: Add a CXL component type (interface)

[Adam Manzanares <a.manzanares@samsung.com>]

On Fri, Mar 18, 2022 at 03:05:50PM +0000, Jonathan Cameron wrote:
> From: Ben Widawsky <ben.widawsky@intel.com>
> 
> A CXL component is a hardware entity that implements CXL component
> registers from the CXL 2.0 spec (8.2.3). Currently these represent 3
> general types.
> 1. Host Bridge
> 2. Ports (root, upstream, downstream)
> 3. Devices (memory, other)
> 
> A CXL component can be conceptually thought of as a PCIe device with
> extra functionality when enumerated and enabled. For this reason, CXL
> does here, and will continue to add on to existing PCI code paths.
> 
> Host bridges will typically need to be handled specially and so they can
> implement this newly introduced interface or not. All other components
> should implement this interface. Implementing this interface allows the
> core PCI code to treat these devices as special where appropriate.
> 
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  hw/pci/pci.c         | 10 ++++++++++
>  include/hw/pci/pci.h |  8 ++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 5cb1232e27..7883778a99 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -201,6 +201,11 @@ static const TypeInfo pci_bus_info = {
>      .class_init = pci_bus_class_init,
>  };
>  
> +static const TypeInfo cxl_interface_info = {
> +    .name          = INTERFACE_CXL_DEVICE,
> +    .parent        = TYPE_INTERFACE,
> +};
> +
>  static const TypeInfo pcie_interface_info = {
>      .name          = INTERFACE_PCIE_DEVICE,
>      .parent        = TYPE_INTERFACE,
> @@ -2182,6 +2187,10 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>          pci_dev->cap_present |= QEMU_PCI_CAP_EXPRESS;
>      }
>  
> +    if (object_class_dynamic_cast(klass, INTERFACE_CXL_DEVICE)) {
> +        pci_dev->cap_present |= QEMU_PCIE_CAP_CXL;
> +    }
> +
>      pci_dev = do_pci_register_device(pci_dev,
>                                       object_get_typename(OBJECT(qdev)),
>                                       pci_dev->devfn, errp);
> @@ -2938,6 +2947,7 @@ static void pci_register_types(void)
>      type_register_static(&pci_bus_info);
>      type_register_static(&pcie_bus_info);
>      type_register_static(&conventional_pci_interface_info);
> +    type_register_static(&cxl_interface_info);
>      type_register_static(&pcie_interface_info);
>      type_register_static(&pci_device_type_info);
>  }
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 3a32b8dd40..98f0d1b844 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -194,6 +194,8 @@ enum {
>      QEMU_PCIE_LNKSTA_DLLLA = (1 << QEMU_PCIE_LNKSTA_DLLLA_BITNR),
>  #define QEMU_PCIE_EXTCAP_INIT_BITNR 9
>      QEMU_PCIE_EXTCAP_INIT = (1 << QEMU_PCIE_EXTCAP_INIT_BITNR),
> +#define QEMU_PCIE_CXL_BITNR 10
> +    QEMU_PCIE_CAP_CXL = (1 << QEMU_PCIE_CXL_BITNR),
>  };
>  
>  #define TYPE_PCI_DEVICE "pci-device"
> @@ -201,6 +203,12 @@ typedef struct PCIDeviceClass PCIDeviceClass;
>  DECLARE_OBJ_CHECKERS(PCIDevice, PCIDeviceClass,
>                       PCI_DEVICE, TYPE_PCI_DEVICE)
>  
> +/*
> + * Implemented by devices that can be plugged on CXL buses. In the spec, this is
> + * actually a "CXL Component, but we name it device to match the PCI naming.
> + */
> +#define INTERFACE_CXL_DEVICE "cxl-device"
> +
>  /* Implemented by devices that can be plugged on PCI Express buses */
>  #define INTERFACE_PCIE_DEVICE "pci-express-device"
>  
> -- 
> 2.32.0
> 
> 

Looks good.

Reviewed by: Adam Manzanares <a.manzanares@samsung.com>