| Message ID | 20201111054724.794888-12-ben.widawsky@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce CXL 2.0 Emulation | expand |
On Tue, 10 Nov 2020 21:47:10 -0800 Ben Widawsky <ben.widawsky@intel.com> wrote: > This works like adding a typical pxb device, except the name is > 'pxb-cxl' instead of 'pxb-pcie'. An example command line would be as > follows: > -device pxb-cxl,id=cxl.0,bus="pcie.0",bus_nr=1 > > A CXL PXB is backward compatible with PCIe. What this means in practice > is that an operating system that is unaware of CXL should still be able > to enumerate this topology as if it were PCIe. > > One can create multiple CXL PXB host bridges, but a host bridge can only > be connected to the main root bus. Host bridges cannot appear elsewhere > in the topology. > > Note that as of this patch, the ACPI tables needed for the host bridge > (specifically, an ACPI object in _SB named ACPI0016 and the CEDT) aren't > created. So while this patch internally creates it, it cannot be > properly used by an operating system or other system software. > > Upcoming patches will allow creating multiple host bridges. > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> Hi Ben, Few minor things inline. Jonathan > --- > hw/pci-bridge/pci_expander_bridge.c | 67 ++++++++++++++++++++++++++++- > hw/pci/pci.c | 7 +++ > include/hw/pci/pci.h | 6 +++ > 3 files changed, 78 insertions(+), 2 deletions(-) > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > index 88c45dc3b5..3a8d815231 100644 > --- a/hw/pci-bridge/pci_expander_bridge.c > +++ b/hw/pci-bridge/pci_expander_bridge.c > @@ -56,6 +56,10 @@ DECLARE_INSTANCE_CHECKER(PXBDev, PXB_DEV, > DECLARE_INSTANCE_CHECKER(PXBDev, PXB_PCIE_DEV, > TYPE_PXB_PCIE_DEVICE) > > +#define TYPE_PXB_CXL_DEVICE "pxb-cxl" > +DECLARE_INSTANCE_CHECKER(PXBDev, PXB_CXL_DEV, > + TYPE_PXB_CXL_DEVICE) > + > struct PXBDev { > /*< private >*/ > PCIDevice parent_obj; > @@ -67,6 +71,11 @@ struct PXBDev { > > static PXBDev *convert_to_pxb(PCIDevice *dev) > { > + /* A CXL PXB's parent bus is PCIe, so the normal check won't work */ > + if (object_dynamic_cast(OBJECT(dev), TYPE_PXB_CXL_DEVICE)) { > + return PXB_CXL_DEV(dev); > + } > + > return pci_bus_is_express(pci_get_bus(dev)) > ? PXB_PCIE_DEV(dev) : PXB_DEV(dev); > } > @@ -111,11 +120,20 @@ static const TypeInfo pxb_pcie_bus_info = { > .class_init = pxb_bus_class_init, > }; > > +static const TypeInfo pxb_cxl_bus_info = { > + .name = TYPE_PXB_CXL_BUS, > + .parent = TYPE_CXL_BUS, > + .instance_size = sizeof(PXBBus), > + .class_init = pxb_bus_class_init, > +}; > + > static const char *pxb_host_root_bus_path(PCIHostState *host_bridge, > PCIBus *rootbus) > { > - PXBBus *bus = pci_bus_is_express(rootbus) ? > - PXB_PCIE_BUS(rootbus) : PXB_BUS(rootbus); > + PXBBus *bus = pci_bus_is_cxl(rootbus) ? > + PXB_CXL_BUS(rootbus) : > + pci_bus_is_express(rootbus) ? PXB_PCIE_BUS(rootbus) : > + PXB_BUS(rootbus); There comes a point where if / else is much more readable. > > snprintf(bus->bus_path, 8, "0000:%02x", pxb_bus_num(rootbus)); > return bus->bus_path; > @@ -380,13 +398,58 @@ static const TypeInfo pxb_pcie_dev_info = { > }, > }; > > +static void pxb_cxl_dev_realize(PCIDevice *dev, Error **errp) > +{ > + /* A CXL PXB's parent bus is still PCIe */ > + if (!pci_bus_is_express(pci_get_bus(dev))) { > + error_setg(errp, "pxb-cxl devices cannot reside on a PCI bus"); > + return; > + } > + > + pxb_dev_realize_common(dev, CXL, errp); > +} > + > +static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > + > + k->realize = pxb_cxl_dev_realize; > + k->exit = pxb_dev_exitfn; > + k->vendor_id = PCI_VENDOR_ID_INTEL; > + k->device_id = 0xabcd; Just to check, is that an officially assigned device_id that we will never have a clash with? Nice ID to get if it is :) > + k->class_id = PCI_CLASS_BRIDGE_HOST; > + k->subsystem_vendor_id = PCI_VENDOR_ID_INTEL; > + > + dc->desc = "CXL Host Bridge"; > + device_class_set_props(dc, pxb_dev_properties); > + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > + > + /* Host bridges aren't hotpluggable. FIXME: spec reference */ > + dc->hotpluggable = false; > +} > + > +static const TypeInfo pxb_cxl_dev_info = { > + .name = TYPE_PXB_CXL_DEVICE, > + .parent = TYPE_PCI_DEVICE, > + .instance_size = sizeof(PXBDev), > + .class_init = pxb_cxl_dev_class_init, > + .interfaces = > + (InterfaceInfo[]){ > + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > + {}, > + }, > +}; > + > static void pxb_register_types(void) > { > type_register_static(&pxb_bus_info); > type_register_static(&pxb_pcie_bus_info); > + type_register_static(&pxb_cxl_bus_info); > type_register_static(&pxb_host_info); > type_register_static(&pxb_dev_info); > type_register_static(&pxb_pcie_dev_info); > + type_register_static(&pxb_cxl_dev_info); > } > > type_init(pxb_register_types) > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index db88788c4b..67eed889a4 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -220,6 +220,12 @@ static const TypeInfo pcie_bus_info = { > .class_init = pcie_bus_class_init, > }; > > +static const TypeInfo cxl_bus_info = { > + .name = TYPE_CXL_BUS, > + .parent = TYPE_PCIE_BUS, > + .class_init = pcie_bus_class_init, > +}; > + > static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num); > static void pci_update_mappings(PCIDevice *d); > static void pci_irq_handler(void *opaque, int irq_num, int level); > @@ -2847,6 +2853,7 @@ static void pci_register_types(void) > { > type_register_static(&pci_bus_info); > type_register_static(&pcie_bus_info); > + type_register_static(&cxl_bus_info); > type_register_static(&conventional_pci_interface_info); > type_register_static(&cxl_interface_info); > type_register_static(&pcie_interface_info); > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index 4e6fd59fdd..52267ff69e 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -405,6 +405,7 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); > #define TYPE_PCI_BUS "PCI" > OBJECT_DECLARE_TYPE(PCIBus, PCIBusClass, PCI_BUS) > #define TYPE_PCIE_BUS "PCIE" > +#define TYPE_CXL_BUS "CXL" > > bool pci_bus_is_express(PCIBus *bus); > > @@ -753,6 +754,11 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev) > pci_irq_deassert(pci_dev); > } > > +static inline int pci_is_cxl(const PCIDevice *d) > +{ > + return d->cap_present & QEMU_PCIE_CAP_CXL; > +} > + > static inline int pci_is_express(const PCIDevice *d) > { > return d->cap_present & QEMU_PCI_CAP_EXPRESS;
On 20-11-16 16:44:09, Jonathan Cameron wrote: > On Tue, 10 Nov 2020 21:47:10 -0800 > Ben Widawsky <ben.widawsky@intel.com> wrote: > > > This works like adding a typical pxb device, except the name is > > 'pxb-cxl' instead of 'pxb-pcie'. An example command line would be as > > follows: > > -device pxb-cxl,id=cxl.0,bus="pcie.0",bus_nr=1 > > > > A CXL PXB is backward compatible with PCIe. What this means in practice > > is that an operating system that is unaware of CXL should still be able > > to enumerate this topology as if it were PCIe. > > > > One can create multiple CXL PXB host bridges, but a host bridge can only > > be connected to the main root bus. Host bridges cannot appear elsewhere > > in the topology. > > > > Note that as of this patch, the ACPI tables needed for the host bridge > > (specifically, an ACPI object in _SB named ACPI0016 and the CEDT) aren't > > created. So while this patch internally creates it, it cannot be > > properly used by an operating system or other system software. > > > > Upcoming patches will allow creating multiple host bridges. > > > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> > Hi Ben, > > Few minor things inline. > > Jonathan > > > --- > > hw/pci-bridge/pci_expander_bridge.c | 67 ++++++++++++++++++++++++++++- > > hw/pci/pci.c | 7 +++ > > include/hw/pci/pci.h | 6 +++ > > 3 files changed, 78 insertions(+), 2 deletions(-) > > > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > > index 88c45dc3b5..3a8d815231 100644 > > --- a/hw/pci-bridge/pci_expander_bridge.c > > +++ b/hw/pci-bridge/pci_expander_bridge.c > > @@ -56,6 +56,10 @@ DECLARE_INSTANCE_CHECKER(PXBDev, PXB_DEV, > > DECLARE_INSTANCE_CHECKER(PXBDev, PXB_PCIE_DEV, > > TYPE_PXB_PCIE_DEVICE) > > > > +#define TYPE_PXB_CXL_DEVICE "pxb-cxl" > > +DECLARE_INSTANCE_CHECKER(PXBDev, PXB_CXL_DEV, > > + TYPE_PXB_CXL_DEVICE) > > + > > struct PXBDev { > > /*< private >*/ > > PCIDevice parent_obj; > > @@ -67,6 +71,11 @@ struct PXBDev { > > > > static PXBDev *convert_to_pxb(PCIDevice *dev) > > { > > + /* A CXL PXB's parent bus is PCIe, so the normal check won't work */ > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PXB_CXL_DEVICE)) { > > + return PXB_CXL_DEV(dev); > > + } > > + > > return pci_bus_is_express(pci_get_bus(dev)) > > ? PXB_PCIE_DEV(dev) : PXB_DEV(dev); > > } > > @@ -111,11 +120,20 @@ static const TypeInfo pxb_pcie_bus_info = { > > .class_init = pxb_bus_class_init, > > }; > > > > +static const TypeInfo pxb_cxl_bus_info = { > > + .name = TYPE_PXB_CXL_BUS, > > + .parent = TYPE_CXL_BUS, > > + .instance_size = sizeof(PXBBus), > > + .class_init = pxb_bus_class_init, > > +}; > > + > > static const char *pxb_host_root_bus_path(PCIHostState *host_bridge, > > PCIBus *rootbus) > > { > > - PXBBus *bus = pci_bus_is_express(rootbus) ? > > - PXB_PCIE_BUS(rootbus) : PXB_BUS(rootbus); > > + PXBBus *bus = pci_bus_is_cxl(rootbus) ? > > + PXB_CXL_BUS(rootbus) : > > + pci_bus_is_express(rootbus) ? PXB_PCIE_BUS(rootbus) : > > + PXB_BUS(rootbus); > > There comes a point where if / else is much more readable. > > > > > snprintf(bus->bus_path, 8, "0000:%02x", pxb_bus_num(rootbus)); > > return bus->bus_path; > > @@ -380,13 +398,58 @@ static const TypeInfo pxb_pcie_dev_info = { > > }, > > }; > > > > +static void pxb_cxl_dev_realize(PCIDevice *dev, Error **errp) > > +{ > > + /* A CXL PXB's parent bus is still PCIe */ > > + if (!pci_bus_is_express(pci_get_bus(dev))) { > > + error_setg(errp, "pxb-cxl devices cannot reside on a PCI bus"); > > + return; > > + } > > + > > + pxb_dev_realize_common(dev, CXL, errp); > > +} > > + > > +static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > + > > + k->realize = pxb_cxl_dev_realize; > > + k->exit = pxb_dev_exitfn; > > + k->vendor_id = PCI_VENDOR_ID_INTEL; > > + k->device_id = 0xabcd; > > Just to check, is that an officially assigned device_id that we will never > have a clash with? Nice ID to get if it is :) No, not the real ID. My understanding is that the host bridge won't exist at all in the PCI hierarchy. So basically all of these can be undeclared. For testing/development purposes I wanted to see this info. Awesomely, it appears if I remove vendor, device, class, and subsystem everything still works and I do not see a bridge device in lspci. So v2 will have this all gone. Thanks. > > > > + k->class_id = PCI_CLASS_BRIDGE_HOST; > > + k->subsystem_vendor_id = PCI_VENDOR_ID_INTEL; > > + > > + dc->desc = "CXL Host Bridge"; > > + device_class_set_props(dc, pxb_dev_properties); > > + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > > + > > + /* Host bridges aren't hotpluggable. FIXME: spec reference */ > > + dc->hotpluggable = false; > > +} > > + > > +static const TypeInfo pxb_cxl_dev_info = { > > + .name = TYPE_PXB_CXL_DEVICE, > > + .parent = TYPE_PCI_DEVICE, > > + .instance_size = sizeof(PXBDev), > > + .class_init = pxb_cxl_dev_class_init, > > + .interfaces = > > + (InterfaceInfo[]){ > > + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > > + {}, > > + }, > > +}; > > + > > static void pxb_register_types(void) > > { > > type_register_static(&pxb_bus_info); > > type_register_static(&pxb_pcie_bus_info); > > + type_register_static(&pxb_cxl_bus_info); > > type_register_static(&pxb_host_info); > > type_register_static(&pxb_dev_info); > > type_register_static(&pxb_pcie_dev_info); > > + type_register_static(&pxb_cxl_dev_info); > > } > > > > type_init(pxb_register_types) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > index db88788c4b..67eed889a4 100644 > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -220,6 +220,12 @@ static const TypeInfo pcie_bus_info = { > > .class_init = pcie_bus_class_init, > > }; > > > > +static const TypeInfo cxl_bus_info = { > > + .name = TYPE_CXL_BUS, > > + .parent = TYPE_PCIE_BUS, > > + .class_init = pcie_bus_class_init, > > +}; > > + > > static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num); > > static void pci_update_mappings(PCIDevice *d); > > static void pci_irq_handler(void *opaque, int irq_num, int level); > > @@ -2847,6 +2853,7 @@ static void pci_register_types(void) > > { > > type_register_static(&pci_bus_info); > > type_register_static(&pcie_bus_info); > > + type_register_static(&cxl_bus_info); > > type_register_static(&conventional_pci_interface_info); > > type_register_static(&cxl_interface_info); > > type_register_static(&pcie_interface_info); > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > > index 4e6fd59fdd..52267ff69e 100644 > > --- a/include/hw/pci/pci.h > > +++ b/include/hw/pci/pci.h > > @@ -405,6 +405,7 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); > > #define TYPE_PCI_BUS "PCI" > > OBJECT_DECLARE_TYPE(PCIBus, PCIBusClass, PCI_BUS) > > #define TYPE_PCIE_BUS "PCIE" > > +#define TYPE_CXL_BUS "CXL" > > > > bool pci_bus_is_express(PCIBus *bus); > > > > @@ -753,6 +754,11 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev) > > pci_irq_deassert(pci_dev); > > } > > > > +static inline int pci_is_cxl(const PCIDevice *d) > > +{ > > + return d->cap_present & QEMU_PCIE_CAP_CXL; > > +} > > + > > static inline int pci_is_express(const PCIDevice *d) > > { > > return d->cap_present & QEMU_PCI_CAP_EXPRESS; >
On Mon, 16 Nov 2020 14:01:40 -0800 Ben Widawsky <ben.widawsky@intel.com> wrote: > On 20-11-16 16:44:09, Jonathan Cameron wrote: > > On Tue, 10 Nov 2020 21:47:10 -0800 > > Ben Widawsky <ben.widawsky@intel.com> wrote: > > > > > This works like adding a typical pxb device, except the name is > > > 'pxb-cxl' instead of 'pxb-pcie'. An example command line would be as > > > follows: > > > -device pxb-cxl,id=cxl.0,bus="pcie.0",bus_nr=1 > > > > > > A CXL PXB is backward compatible with PCIe. What this means in practice > > > is that an operating system that is unaware of CXL should still be able > > > to enumerate this topology as if it were PCIe. > > > > > > One can create multiple CXL PXB host bridges, but a host bridge can only > > > be connected to the main root bus. Host bridges cannot appear elsewhere > > > in the topology. > > > > > > Note that as of this patch, the ACPI tables needed for the host bridge > > > (specifically, an ACPI object in _SB named ACPI0016 and the CEDT) aren't > > > created. So while this patch internally creates it, it cannot be > > > properly used by an operating system or other system software. > > > > > > Upcoming patches will allow creating multiple host bridges. > > > > > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> > > Hi Ben, > > > > Few minor things inline. > > > > Jonathan > > > > > --- > > > hw/pci-bridge/pci_expander_bridge.c | 67 ++++++++++++++++++++++++++++- > > > hw/pci/pci.c | 7 +++ > > > include/hw/pci/pci.h | 6 +++ > > > 3 files changed, 78 insertions(+), 2 deletions(-) > > > > > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > > > index 88c45dc3b5..3a8d815231 100644 > > > --- a/hw/pci-bridge/pci_expander_bridge.c > > > +++ b/hw/pci-bridge/pci_expander_bridge.c > > > @@ -56,6 +56,10 @@ DECLARE_INSTANCE_CHECKER(PXBDev, PXB_DEV, > > > DECLARE_INSTANCE_CHECKER(PXBDev, PXB_PCIE_DEV, > > > TYPE_PXB_PCIE_DEVICE) > > > > > > +#define TYPE_PXB_CXL_DEVICE "pxb-cxl" > > > +DECLARE_INSTANCE_CHECKER(PXBDev, PXB_CXL_DEV, > > > + TYPE_PXB_CXL_DEVICE) > > > + > > > struct PXBDev { > > > /*< private >*/ > > > PCIDevice parent_obj; > > > @@ -67,6 +71,11 @@ struct PXBDev { > > > > > > static PXBDev *convert_to_pxb(PCIDevice *dev) > > > { > > > + /* A CXL PXB's parent bus is PCIe, so the normal check won't work */ > > > + if (object_dynamic_cast(OBJECT(dev), TYPE_PXB_CXL_DEVICE)) { > > > + return PXB_CXL_DEV(dev); > > > + } > > > + > > > return pci_bus_is_express(pci_get_bus(dev)) > > > ? PXB_PCIE_DEV(dev) : PXB_DEV(dev); > > > } > > > @@ -111,11 +120,20 @@ static const TypeInfo pxb_pcie_bus_info = { > > > .class_init = pxb_bus_class_init, > > > }; > > > > > > +static const TypeInfo pxb_cxl_bus_info = { > > > + .name = TYPE_PXB_CXL_BUS, > > > + .parent = TYPE_CXL_BUS, > > > + .instance_size = sizeof(PXBBus), > > > + .class_init = pxb_bus_class_init, > > > +}; > > > + > > > static const char *pxb_host_root_bus_path(PCIHostState *host_bridge, > > > PCIBus *rootbus) > > > { > > > - PXBBus *bus = pci_bus_is_express(rootbus) ? > > > - PXB_PCIE_BUS(rootbus) : PXB_BUS(rootbus); > > > + PXBBus *bus = pci_bus_is_cxl(rootbus) ? > > > + PXB_CXL_BUS(rootbus) : > > > + pci_bus_is_express(rootbus) ? PXB_PCIE_BUS(rootbus) : > > > + PXB_BUS(rootbus); > > > > There comes a point where if / else is much more readable. > > > > > > > > snprintf(bus->bus_path, 8, "0000:%02x", pxb_bus_num(rootbus)); > > > return bus->bus_path; > > > @@ -380,13 +398,58 @@ static const TypeInfo pxb_pcie_dev_info = { > > > }, > > > }; > > > > > > +static void pxb_cxl_dev_realize(PCIDevice *dev, Error **errp) > > > +{ > > > + /* A CXL PXB's parent bus is still PCIe */ > > > + if (!pci_bus_is_express(pci_get_bus(dev))) { > > > + error_setg(errp, "pxb-cxl devices cannot reside on a PCI bus"); > > > + return; > > > + } > > > + > > > + pxb_dev_realize_common(dev, CXL, errp); > > > +} > > > + > > > +static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data) > > > +{ > > > + DeviceClass *dc = DEVICE_CLASS(klass); > > > + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > > + > > > + k->realize = pxb_cxl_dev_realize; > > > + k->exit = pxb_dev_exitfn; > > > + k->vendor_id = PCI_VENDOR_ID_INTEL; > > > + k->device_id = 0xabcd; > > > > Just to check, is that an officially assigned device_id that we will never > > have a clash with? Nice ID to get if it is :) > > No, not the real ID. > > My understanding is that the host bridge won't exist at all in the PCI > hierarchy. So basically all of these can be undeclared. For testing/development > purposes I wanted to see this info. > > Awesomely, it appears if I remove vendor, device, class, and subsystem > everything still works and I do not see a bridge device in lspci. So v2 will > have this all gone. Pity - it is always fun to track down the holder of the magic list of device IDs and try to explain to them why you want one for a device that doesn't "exist" and isn't visible anyway. I guess no ID is less confusing :) Jonathan > > Thanks. > > > > > > > > + k->class_id = PCI_CLASS_BRIDGE_HOST; > > > + k->subsystem_vendor_id = PCI_VENDOR_ID_INTEL; > > > + > > > + dc->desc = "CXL Host Bridge"; > > > + device_class_set_props(dc, pxb_dev_properties); > > > + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > > > + > > > + /* Host bridges aren't hotpluggable. FIXME: spec reference */ > > > + dc->hotpluggable = false; > > > +} > > > + > > > +static const TypeInfo pxb_cxl_dev_info = { > > > + .name = TYPE_PXB_CXL_DEVICE, > > > + .parent = TYPE_PCI_DEVICE, > > > + .instance_size = sizeof(PXBDev), > > > + .class_init = pxb_cxl_dev_class_init, > > > + .interfaces = > > > + (InterfaceInfo[]){ > > > + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > > > + {}, > > > + }, > > > +}; > > > + > > > static void pxb_register_types(void) > > > { > > > type_register_static(&pxb_bus_info); > > > type_register_static(&pxb_pcie_bus_info); > > > + type_register_static(&pxb_cxl_bus_info); > > > type_register_static(&pxb_host_info); > > > type_register_static(&pxb_dev_info); > > > type_register_static(&pxb_pcie_dev_info); > > > + type_register_static(&pxb_cxl_dev_info); > > > } > > > > > > type_init(pxb_register_types) > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > > index db88788c4b..67eed889a4 100644 > > > --- a/hw/pci/pci.c > > > +++ b/hw/pci/pci.c > > > @@ -220,6 +220,12 @@ static const TypeInfo pcie_bus_info = { > > > .class_init = pcie_bus_class_init, > > > }; > > > > > > +static const TypeInfo cxl_bus_info = { > > > + .name = TYPE_CXL_BUS, > > > + .parent = TYPE_PCIE_BUS, > > > + .class_init = pcie_bus_class_init, > > > +}; > > > + > > > static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num); > > > static void pci_update_mappings(PCIDevice *d); > > > static void pci_irq_handler(void *opaque, int irq_num, int level); > > > @@ -2847,6 +2853,7 @@ static void pci_register_types(void) > > > { > > > type_register_static(&pci_bus_info); > > > type_register_static(&pcie_bus_info); > > > + type_register_static(&cxl_bus_info); > > > type_register_static(&conventional_pci_interface_info); > > > type_register_static(&cxl_interface_info); > > > type_register_static(&pcie_interface_info); > > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > > > index 4e6fd59fdd..52267ff69e 100644 > > > --- a/include/hw/pci/pci.h > > > +++ b/include/hw/pci/pci.h > > > @@ -405,6 +405,7 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); > > > #define TYPE_PCI_BUS "PCI" > > > OBJECT_DECLARE_TYPE(PCIBus, PCIBusClass, PCI_BUS) > > > #define TYPE_PCIE_BUS "PCIE" > > > +#define TYPE_CXL_BUS "CXL" > > > > > > bool pci_bus_is_express(PCIBus *bus); > > > > > > @@ -753,6 +754,11 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev) > > > pci_irq_deassert(pci_dev); > > > } > > > > > > +static inline int pci_is_cxl(const PCIDevice *d) > > > +{ > > > + return d->cap_present & QEMU_PCIE_CAP_CXL; > > > +} > > > + > > > static inline int pci_is_express(const PCIDevice *d) > > > { > > > return d->cap_present & QEMU_PCI_CAP_EXPRESS; > >
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index 88c45dc3b5..3a8d815231 100644 --- a/hw/pci-bridge/pci_expander_bridge.c +++ b/hw/pci-bridge/pci_expander_bridge.c @@ -56,6 +56,10 @@ DECLARE_INSTANCE_CHECKER(PXBDev, PXB_DEV, DECLARE_INSTANCE_CHECKER(PXBDev, PXB_PCIE_DEV, TYPE_PXB_PCIE_DEVICE) +#define TYPE_PXB_CXL_DEVICE "pxb-cxl" +DECLARE_INSTANCE_CHECKER(PXBDev, PXB_CXL_DEV, + TYPE_PXB_CXL_DEVICE) + struct PXBDev { /*< private >*/ PCIDevice parent_obj; @@ -67,6 +71,11 @@ struct PXBDev { static PXBDev *convert_to_pxb(PCIDevice *dev) { + /* A CXL PXB's parent bus is PCIe, so the normal check won't work */ + if (object_dynamic_cast(OBJECT(dev), TYPE_PXB_CXL_DEVICE)) { + return PXB_CXL_DEV(dev); + } + return pci_bus_is_express(pci_get_bus(dev)) ? PXB_PCIE_DEV(dev) : PXB_DEV(dev); } @@ -111,11 +120,20 @@ static const TypeInfo pxb_pcie_bus_info = { .class_init = pxb_bus_class_init, }; +static const TypeInfo pxb_cxl_bus_info = { + .name = TYPE_PXB_CXL_BUS, + .parent = TYPE_CXL_BUS, + .instance_size = sizeof(PXBBus), + .class_init = pxb_bus_class_init, +}; + static const char *pxb_host_root_bus_path(PCIHostState *host_bridge, PCIBus *rootbus) { - PXBBus *bus = pci_bus_is_express(rootbus) ? - PXB_PCIE_BUS(rootbus) : PXB_BUS(rootbus); + PXBBus *bus = pci_bus_is_cxl(rootbus) ? + PXB_CXL_BUS(rootbus) : + pci_bus_is_express(rootbus) ? PXB_PCIE_BUS(rootbus) : + PXB_BUS(rootbus); snprintf(bus->bus_path, 8, "0000:%02x", pxb_bus_num(rootbus)); return bus->bus_path; @@ -380,13 +398,58 @@ static const TypeInfo pxb_pcie_dev_info = { }, }; +static void pxb_cxl_dev_realize(PCIDevice *dev, Error **errp) +{ + /* A CXL PXB's parent bus is still PCIe */ + if (!pci_bus_is_express(pci_get_bus(dev))) { + error_setg(errp, "pxb-cxl devices cannot reside on a PCI bus"); + return; + } + + pxb_dev_realize_common(dev, CXL, errp); +} + +static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + + k->realize = pxb_cxl_dev_realize; + k->exit = pxb_dev_exitfn; + k->vendor_id = PCI_VENDOR_ID_INTEL; + k->device_id = 0xabcd; + k->class_id = PCI_CLASS_BRIDGE_HOST; + k->subsystem_vendor_id = PCI_VENDOR_ID_INTEL; + + dc->desc = "CXL Host Bridge"; + device_class_set_props(dc, pxb_dev_properties); + set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); + + /* Host bridges aren't hotpluggable. FIXME: spec reference */ + dc->hotpluggable = false; +} + +static const TypeInfo pxb_cxl_dev_info = { + .name = TYPE_PXB_CXL_DEVICE, + .parent = TYPE_PCI_DEVICE, + .instance_size = sizeof(PXBDev), + .class_init = pxb_cxl_dev_class_init, + .interfaces = + (InterfaceInfo[]){ + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, + {}, + }, +}; + static void pxb_register_types(void) { type_register_static(&pxb_bus_info); type_register_static(&pxb_pcie_bus_info); + type_register_static(&pxb_cxl_bus_info); type_register_static(&pxb_host_info); type_register_static(&pxb_dev_info); type_register_static(&pxb_pcie_dev_info); + type_register_static(&pxb_cxl_dev_info); } type_init(pxb_register_types) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index db88788c4b..67eed889a4 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -220,6 +220,12 @@ static const TypeInfo pcie_bus_info = { .class_init = pcie_bus_class_init, }; +static const TypeInfo cxl_bus_info = { + .name = TYPE_CXL_BUS, + .parent = TYPE_PCIE_BUS, + .class_init = pcie_bus_class_init, +}; + static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num); static void pci_update_mappings(PCIDevice *d); static void pci_irq_handler(void *opaque, int irq_num, int level); @@ -2847,6 +2853,7 @@ static void pci_register_types(void) { type_register_static(&pci_bus_info); type_register_static(&pcie_bus_info); + type_register_static(&cxl_bus_info); type_register_static(&conventional_pci_interface_info); type_register_static(&cxl_interface_info); type_register_static(&pcie_interface_info); diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index 4e6fd59fdd..52267ff69e 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -405,6 +405,7 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin); #define TYPE_PCI_BUS "PCI" OBJECT_DECLARE_TYPE(PCIBus, PCIBusClass, PCI_BUS) #define TYPE_PCIE_BUS "PCIE" +#define TYPE_CXL_BUS "CXL" bool pci_bus_is_express(PCIBus *bus); @@ -753,6 +754,11 @@ static inline void pci_irq_pulse(PCIDevice *pci_dev) pci_irq_deassert(pci_dev); } +static inline int pci_is_cxl(const PCIDevice *d) +{ + return d->cap_present & QEMU_PCIE_CAP_CXL; +} + static inline int pci_is_express(const PCIDevice *d) { return d->cap_present & QEMU_PCI_CAP_EXPRESS;
This works like adding a typical pxb device, except the name is 'pxb-cxl' instead of 'pxb-pcie'. An example command line would be as follows: -device pxb-cxl,id=cxl.0,bus="pcie.0",bus_nr=1 A CXL PXB is backward compatible with PCIe. What this means in practice is that an operating system that is unaware of CXL should still be able to enumerate this topology as if it were PCIe. One can create multiple CXL PXB host bridges, but a host bridge can only be connected to the main root bus. Host bridges cannot appear elsewhere in the topology. Note that as of this patch, the ACPI tables needed for the host bridge (specifically, an ACPI object in _SB named ACPI0016 and the CEDT) aren't created. So while this patch internally creates it, it cannot be properly used by an operating system or other system software. Upcoming patches will allow creating multiple host bridges. Signed-off-by: Ben Widawsky <ben.widawsky@intel.com> --- hw/pci-bridge/pci_expander_bridge.c | 67 ++++++++++++++++++++++++++++- hw/pci/pci.c | 7 +++ include/hw/pci/pci.h | 6 +++ 3 files changed, 78 insertions(+), 2 deletions(-)