On 21-02-02 19:21:35, Jonathan Cameron wrote:
> On Mon, 1 Feb 2021 16:59:34 -0800
> Ben Widawsky <ben.widaw...@intel.com> wrote:
>
> > CXL host bridges themselves may have MMIO. Since host bridges don't have
> > a BAR they are treated as special for MMIO.
> >
> > Signed-off-by: Ben Widawsky <ben.widaw...@intel.com>
> >
> > --
> >
> > It's arbitrarily chosen here to pick 0xD0000000 as the base for the host
> > bridge MMIO. I'm not sure what the right way to find free space for
> > platform hardcoded things like this is.
>
> Seems like this needs to come from the machine definition.
> This is fairly easy for arm/virt, where there is a clearly laid out memory
> map.
> For hw/i386 I'm less sure on how to do it.
I think this is how to do it :D
>
> Having said that, for this particular magic device, we do have a PCI EP
> associated with it. How about putting all the host bridge MMIO into a
> BAR of that rather than having it separate.
> That has the added advantage of making it discoverable from firmware.
>
> Any normal system is going to have this is impdef for discovery anyway.
>
This is not how it's expected to work for Intel at least. If the device was
discoverable you wouldn't need CEDT/CHBS. The magic host bridges are only
advertised via the CEDT.
When I build and run QEMU for x86_64, I do not see the host bridge in the pci
topology, do you (it's meant to not be there)?
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
...
34:00.0 PCI bridge: Intel Corporation Device 7075
35:00.0 Memory controller [0502]: Intel Corporation Device 0d93 (rev 01)
That's Q35, Root Port, and Type 3 device respectively.
> That would then let you drop the separate definition of CXLHost structure
> though it needs a bit of figuring out what to do with the memory window
> setup etc.
>
> I tried hacking it together, but not gotten it working yet.
>
> > ---
> > hw/pci-bridge/pci_expander_bridge.c | 53 ++++++++++++++++++++++++++++-
> > include/hw/cxl/cxl.h | 2 ++
> > 2 files changed, 54 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c
> > b/hw/pci-bridge/pci_expander_bridge.c
> > index 5021b60435..226a8a5fff 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -17,6 +17,7 @@
> > #include "hw/pci/pci_host.h"
> > #include "hw/qdev-properties.h"
> > #include "hw/pci/pci_bridge.h"
> > +#include "hw/cxl/cxl.h"
> > #include "qemu/range.h"
> > #include "qemu/error-report.h"
> > #include "qemu/module.h"
> > @@ -70,6 +71,12 @@ struct PXBDev {
> > int32_t uid;
> > };
> >
> > +typedef struct CXLHost {
> > + PCIHostState parent_obj;
> > +
> > + CXLComponentState cxl_cstate;
> > +} CXLHost;
> > +
> > static PXBDev *convert_to_pxb(PCIDevice *dev)
> > {
> > /* A CXL PXB's parent bus is PCIe, so the normal check won't work */
> > @@ -85,6 +92,9 @@ static GList *pxb_dev_list;
> >
> > #define TYPE_PXB_HOST "pxb-host"
> >
> > +#define TYPE_PXB_CXL_HOST "pxb-cxl-host"
> > +#define PXB_CXL_HOST(obj) OBJECT_CHECK(CXLHost, (obj), TYPE_PXB_CXL_HOST)
> > +
> > static int pxb_bus_num(PCIBus *bus)
> > {
> > PXBDev *pxb = convert_to_pxb(bus->parent_dev);
> > @@ -198,6 +208,46 @@ static const TypeInfo pxb_host_info = {
> > .class_init = pxb_host_class_init,
> > };
> >
> > +static void pxb_cxl_realize(DeviceState *dev, Error **errp)
> > +{
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > + PCIHostState *phb = PCI_HOST_BRIDGE(dev);
> > + CXLHost *cxl = PXB_CXL_HOST(dev);
> > + CXLComponentState *cxl_cstate = &cxl->cxl_cstate;
> > + struct MemoryRegion *mr = &cxl_cstate->crb.component_registers;
> > +
> > + cxl_component_register_block_init(OBJECT(dev), cxl_cstate,
> > + TYPE_PXB_CXL_HOST);
> > + sysbus_init_mmio(sbd, mr);
> > +
> > + /* FIXME: support multiple host bridges. */
> > + sysbus_mmio_map(sbd, 0, CXL_HOST_BASE +
> > + memory_region_size(mr) *
> > pci_bus_uid(phb->bus));
> > +}
> > +
> > +static void pxb_cxl_host_class_init(ObjectClass *class, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(class);
> > + PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
> > +
> > + hc->root_bus_path = pxb_host_root_bus_path;
> > + dc->fw_name = "cxl";
> > + dc->realize = pxb_cxl_realize;
> > + /* Reason: Internal part of the pxb/pxb-pcie device, not usable by
> > itself */
> > + dc->user_creatable = false;
> > +}
> > +
> > +/*
> > + * This is a device to handle the MMIO for a CXL host bridge. It does
> > nothing
> > + * else.
> > + */
> > +static const TypeInfo cxl_host_info = {
> > + .name = TYPE_PXB_CXL_HOST,
> > + .parent = TYPE_PCI_HOST_BRIDGE,
> > + .instance_size = sizeof(CXLHost),
> > + .class_init = pxb_cxl_host_class_init,
> > +};
> > +
> > /*
> > * Registers the PXB bus as a child of pci host root bus.
> > */
> > @@ -272,7 +322,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, enum
> > BusType type,
> > dev_name = dev->qdev.id;
> > }
> >
> > - ds = qdev_new(TYPE_PXB_HOST);
> > + ds = qdev_new(type == CXL ? TYPE_PXB_CXL_HOST : TYPE_PXB_HOST);
> > if (type == PCIE) {
> > bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0,
> > TYPE_PXB_PCIE_BUS);
> > } else if (type == CXL) {
> > @@ -466,6 +516,7 @@ static void pxb_register_types(void)
> > type_register_static(&pxb_pcie_bus_info);
> > type_register_static(&pxb_cxl_bus_info);
> > type_register_static(&pxb_host_info);
> > + type_register_static(&cxl_host_info);
> > type_register_static(&pxb_dev_info);
> > type_register_static(&pxb_pcie_dev_info);
> > type_register_static(&pxb_cxl_dev_info);
> > diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> > index 362cda40de..6bc344f205 100644
> > --- a/include/hw/cxl/cxl.h
> > +++ b/include/hw/cxl/cxl.h
> > @@ -17,5 +17,7 @@
> > #define COMPONENT_REG_BAR_IDX 0
> > #define DEVICE_REG_BAR_IDX 2
> >
> > +#define CXL_HOST_BASE 0xD0000000
> > +
> > #endif
> >
>
>
