* [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation @ 2023-01-23 12:17 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2023-01-23 12:17 UTC (permalink / raw) To: qemu-devel Cc: Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Fan Ni Until now, testing using CXL has relied up always using two root ports below a host bridge, to work around a current assumption in the Linux kernel support that, in the single root port case, the implementation will use the allowed passthrough decoder implementation choice. If that choice is made all accesses are routed from the host bridge to the single root port that is present. Effectively we have a pass through decoder (it is called that in the kernel driver). This patch series implements that functionality and makes it the default See patch 2 for a discussion of why I think we can make this change without backwards compatibility issues (basically if it didn't work before who are we breaking by making it work?) Whilst this limitation has been known since the initial QEMU patch postings / kernel CXL region support, Fan Ni Ran into it recently reminding me that we should solve it. https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/ Tree with a large set of patches before this at: https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20 I've done some basic testing, though I did hit what appears to be a kernel race on region bring up of existing region / namespace in a 1HB 2RP 2EP test case. That is proving hard to replicate consistently but doesn't seem to have anything to do with the emulation other than perhaps we are opening up a race by responding slowly to something. Jonathan Cameron (2): hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers hw/pxb-cxl: Support passthrough HDM Decoders unless overridden hw/cxl/cxl-host.c | 31 +++++++++++++-------- hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ include/hw/cxl/cxl.h | 1 + include/hw/cxl/cxl_component.h | 1 + include/hw/pci/pci_bridge.h | 1 + include/hw/pci/pcie_port.h | 2 ++ 7 files changed, 100 insertions(+), 17 deletions(-) -- 2.37.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation @ 2023-01-23 12:17 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron via @ 2023-01-23 12:17 UTC (permalink / raw) To: qemu-devel Cc: Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Fan Ni Until now, testing using CXL has relied up always using two root ports below a host bridge, to work around a current assumption in the Linux kernel support that, in the single root port case, the implementation will use the allowed passthrough decoder implementation choice. If that choice is made all accesses are routed from the host bridge to the single root port that is present. Effectively we have a pass through decoder (it is called that in the kernel driver). This patch series implements that functionality and makes it the default See patch 2 for a discussion of why I think we can make this change without backwards compatibility issues (basically if it didn't work before who are we breaking by making it work?) Whilst this limitation has been known since the initial QEMU patch postings / kernel CXL region support, Fan Ni Ran into it recently reminding me that we should solve it. https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/ Tree with a large set of patches before this at: https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20 I've done some basic testing, though I did hit what appears to be a kernel race on region bring up of existing region / namespace in a 1HB 2RP 2EP test case. That is proving hard to replicate consistently but doesn't seem to have anything to do with the emulation other than perhaps we are opening up a race by responding slowly to something. Jonathan Cameron (2): hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers hw/pxb-cxl: Support passthrough HDM Decoders unless overridden hw/cxl/cxl-host.c | 31 +++++++++++++-------- hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ include/hw/cxl/cxl.h | 1 + include/hw/cxl/cxl_component.h | 1 + include/hw/pci/pci_bridge.h | 1 + include/hw/pci/pcie_port.h | 2 ++ 7 files changed, 100 insertions(+), 17 deletions(-) -- 2.37.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH 1/2] hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers 2023-01-23 12:17 ` Jonathan Cameron via @ 2023-01-23 12:17 ` Jonathan Cameron via -1 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2023-01-23 12:17 UTC (permalink / raw) To: qemu-devel Cc: Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Fan Ni These two helpers enable host bridges to operate differently depending on the number of downstream ports, in particular if there is only a single port. Useful for CXL where HDM address decoders are allowed to be implicit in the host bridge if there is only a single root port. Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- hw/pci/pcie_port.c | 38 ++++++++++++++++++++++++++++++++++++++ include/hw/pci/pcie_port.h | 2 ++ 2 files changed, 40 insertions(+) diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c index 687e4e763a..b709777e0c 100644 --- a/hw/pci/pcie_port.c +++ b/hw/pci/pcie_port.c @@ -161,6 +161,44 @@ PCIDevice *pcie_find_port_by_pn(PCIBus *bus, uint8_t pn) return NULL; } +/* Find first port in devfn number */ +PCIDevice *pcie_find_port_first(PCIBus *bus) +{ + int devfn; + + for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) { + PCIDevice *d = bus->devices[devfn]; + + if (!d || !pci_is_express(d) || !d->exp.exp_cap) { + continue; + } + + if (object_dynamic_cast(OBJECT(d), TYPE_PCIE_PORT)) { + return d; + } + } + + return NULL; +} + +int pcie_count_ds_ports(PCIBus *bus) +{ + int dsp_count = 0; + int devfn; + + for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) { + PCIDevice *d = bus->devices[devfn]; + + if (!d || !pci_is_express(d) || !d->exp.exp_cap) { + continue; + } + if (object_dynamic_cast(OBJECT(d), TYPE_PCIE_PORT)) { + dsp_count++; + } + } + return dsp_count; +} + static const TypeInfo pcie_port_type_info = { .name = TYPE_PCIE_PORT, .parent = TYPE_PCI_BRIDGE, diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h index fd484afb30..2cbad72555 100644 --- a/include/hw/pci/pcie_port.h +++ b/include/hw/pci/pcie_port.h @@ -41,6 +41,8 @@ struct PCIEPort { void pcie_port_init_reg(PCIDevice *d); PCIDevice *pcie_find_port_by_pn(PCIBus *bus, uint8_t pn); +PCIDevice *pcie_find_port_first(PCIBus *bus); +int pcie_count_ds_ports(PCIBus *bus); #define TYPE_PCIE_SLOT "pcie-slot" OBJECT_DECLARE_SIMPLE_TYPE(PCIESlot, PCIE_SLOT) -- 2.37.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH 1/2] hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers @ 2023-01-23 12:17 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron via @ 2023-01-23 12:17 UTC (permalink / raw) To: qemu-devel Cc: Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Fan Ni These two helpers enable host bridges to operate differently depending on the number of downstream ports, in particular if there is only a single port. Useful for CXL where HDM address decoders are allowed to be implicit in the host bridge if there is only a single root port. Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- hw/pci/pcie_port.c | 38 ++++++++++++++++++++++++++++++++++++++ include/hw/pci/pcie_port.h | 2 ++ 2 files changed, 40 insertions(+) diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c index 687e4e763a..b709777e0c 100644 --- a/hw/pci/pcie_port.c +++ b/hw/pci/pcie_port.c @@ -161,6 +161,44 @@ PCIDevice *pcie_find_port_by_pn(PCIBus *bus, uint8_t pn) return NULL; } +/* Find first port in devfn number */ +PCIDevice *pcie_find_port_first(PCIBus *bus) +{ + int devfn; + + for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) { + PCIDevice *d = bus->devices[devfn]; + + if (!d || !pci_is_express(d) || !d->exp.exp_cap) { + continue; + } + + if (object_dynamic_cast(OBJECT(d), TYPE_PCIE_PORT)) { + return d; + } + } + + return NULL; +} + +int pcie_count_ds_ports(PCIBus *bus) +{ + int dsp_count = 0; + int devfn; + + for (devfn = 0; devfn < ARRAY_SIZE(bus->devices); devfn++) { + PCIDevice *d = bus->devices[devfn]; + + if (!d || !pci_is_express(d) || !d->exp.exp_cap) { + continue; + } + if (object_dynamic_cast(OBJECT(d), TYPE_PCIE_PORT)) { + dsp_count++; + } + } + return dsp_count; +} + static const TypeInfo pcie_port_type_info = { .name = TYPE_PCIE_PORT, .parent = TYPE_PCI_BRIDGE, diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h index fd484afb30..2cbad72555 100644 --- a/include/hw/pci/pcie_port.h +++ b/include/hw/pci/pcie_port.h @@ -41,6 +41,8 @@ struct PCIEPort { void pcie_port_init_reg(PCIDevice *d); PCIDevice *pcie_find_port_by_pn(PCIBus *bus, uint8_t pn); +PCIDevice *pcie_find_port_first(PCIBus *bus); +int pcie_count_ds_ports(PCIBus *bus); #define TYPE_PCIE_SLOT "pcie-slot" OBJECT_DECLARE_SIMPLE_TYPE(PCIESlot, PCIE_SLOT) -- 2.37.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH 2/2] hw/pxb-cxl: Support passthrough HDM Decoders unless overridden 2023-01-23 12:17 ` Jonathan Cameron via @ 2023-01-23 12:17 ` Jonathan Cameron via -1 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2023-01-23 12:17 UTC (permalink / raw) To: qemu-devel Cc: Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Fan Ni The CXL r3.0 specification allows for there to be no HDM decoders on CXL Host Bridges if they have only a single root port. Instead, all accesses directed to the host bridge (as specified in CXL Fixed Memory Windows) are assumed to be routed to the single root port. Linux currently assumes this implementation choice. So to simplify testing, make QEMU emulation also default to no HDM decoders under these particular circumstances, but provide a hdm_for_passthrough boolean option to have HDM decoders as previously. Technically this is breaking backwards compatibility, but given the only known software stack used with the QEMU emulation is the Linux kernel and this configuration did not work before this change, there are unlikely to be any complaints that it now works. The option is retained to allow testing of software that does allow for these HDM decoders to exist, once someone writes it. Reported-by: Fan Ni <fan.ni@samsung.com> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- hw/cxl/cxl-host.c | 31 +++++++++++++-------- hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- include/hw/cxl/cxl.h | 1 + include/hw/cxl/cxl_component.h | 1 + include/hw/pci/pci_bridge.h | 1 + 5 files changed, 60 insertions(+), 17 deletions(-) diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c index 3c1ec8732a..6e923ceeaf 100644 --- a/hw/cxl/cxl-host.c +++ b/hw/cxl/cxl-host.c @@ -146,21 +146,28 @@ static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow *fw, hwaddr addr) return NULL; } - hb_cstate = cxl_get_hb_cstate(hb); - if (!hb_cstate) { - return NULL; - } + if (cxl_get_hb_passthrough(hb)) { + rp = pcie_find_port_first(hb->bus); + if (!rp) { + return NULL; + } + } else { + hb_cstate = cxl_get_hb_cstate(hb); + if (!hb_cstate) { + return NULL; + } - cache_mem = hb_cstate->crb.cache_mem_registers; + cache_mem = hb_cstate->crb.cache_mem_registers; - target_found = cxl_hdm_find_target(cache_mem, addr, &target); - if (!target_found) { - return NULL; - } + target_found = cxl_hdm_find_target(cache_mem, addr, &target); + if (!target_found) { + return NULL; + } - rp = pcie_find_port_by_pn(hb->bus, target); - if (!rp) { - return NULL; + rp = pcie_find_port_by_pn(hb->bus, target); + if (!rp) { + return NULL; + } } d = pci_bridge_get_sec_bus(PCI_BRIDGE(rp))->devices[0]; diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index e752a21292..e2aabc12aa 100644 --- a/hw/pci-bridge/pci_expander_bridge.c +++ b/hw/pci-bridge/pci_expander_bridge.c @@ -15,6 +15,7 @@ #include "hw/pci/pci.h" #include "hw/pci/pci_bus.h" #include "hw/pci/pci_host.h" +#include "hw/pci/pcie_port.h" #include "hw/qdev-properties.h" #include "hw/pci/pci_bridge.h" #include "hw/pci-bridge/pci_expander_bridge.h" @@ -79,6 +80,13 @@ CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb) return &host->cxl_cstate; } +bool cxl_get_hb_passthrough(PCIHostState *hb) +{ + CXLHost *host = PXB_CXL_HOST(hb); + + return host->passthrough; +} + static int pxb_bus_num(PCIBus *bus) { PXBDev *pxb = convert_to_pxb(bus->parent_dev); @@ -289,15 +297,31 @@ static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin) return pin - PCI_SLOT(pxb->devfn); } -static void pxb_dev_reset(DeviceState *dev) +static void pxb_cxl_dev_reset(DeviceState *dev) { CXLHost *cxl = PXB_CXL_DEV(dev)->cxl.cxl_host_bridge; CXLComponentState *cxl_cstate = &cxl->cxl_cstate; + PCIHostState *hb = PCI_HOST_BRIDGE(cxl); uint32_t *reg_state = cxl_cstate->crb.cache_mem_registers; uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask; + int dsp_count = 0; cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT); - ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT, 8); + /* + * The CXL specification allows for host bridges with no HDM decoders + * if they only have a single root port. + */ + if (!PXB_DEV(dev)->hdm_for_passthrough) { + dsp_count = pcie_count_ds_ports(hb->bus); + } + /* Initial reset will have 0 dsp so wait until > 0 */ + if (dsp_count == 1) { + cxl->passthrough = true; + /* Set Capability ID in header to NONE */ + ARRAY_FIELD_DP32(reg_state, CXL_HDM_CAPABILITY_HEADER, ID, 0); + } else { + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT, 8); + } } static gint pxb_compare(gconstpointer a, gconstpointer b) @@ -481,9 +505,18 @@ static void pxb_cxl_dev_realize(PCIDevice *dev, Error **errp) } pxb_dev_realize_common(dev, CXL, errp); - pxb_dev_reset(DEVICE(dev)); + pxb_cxl_dev_reset(DEVICE(dev)); } +static Property pxb_cxl_dev_properties[] = { + /* Note: 0 is not a legal PXB bus number. */ + DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0), + DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, NUMA_NODE_UNASSIGNED), + DEFINE_PROP_BOOL("bypass_iommu", PXBDev, bypass_iommu, false), + DEFINE_PROP_BOOL("hdm_for_passthrough", PXBDev, hdm_for_passthrough, false), + DEFINE_PROP_END_OF_LIST(), +}; + static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -497,12 +530,12 @@ static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data) */ dc->desc = "CXL Host Bridge"; - device_class_set_props(dc, pxb_dev_properties); + device_class_set_props(dc, pxb_cxl_dev_properties); set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); /* Host bridges aren't hotpluggable. FIXME: spec reference */ dc->hotpluggable = false; - dc->reset = pxb_dev_reset; + dc->reset = pxb_cxl_dev_reset; } static const TypeInfo pxb_cxl_dev_info = { diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h index 716f0f0f2a..9c8185ae88 100644 --- a/include/hw/cxl/cxl.h +++ b/include/hw/cxl/cxl.h @@ -50,6 +50,7 @@ struct CXLHost { PCIHostState parent_obj; CXLComponentState cxl_cstate; + bool passthrough; }; #define TYPE_PXB_CXL_HOST "pxb-cxl-host" diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h index 870faeb6dd..97e45e6747 100644 --- a/include/hw/cxl/cxl_component.h +++ b/include/hw/cxl/cxl_component.h @@ -251,6 +251,7 @@ static inline hwaddr cxl_decode_ig(int ig) } CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb); +bool cxl_get_hb_passthrough(PCIHostState *hb); void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); void cxl_doe_cdat_release(CXLComponentState *cxl_cstate); diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h index 63a7521567..81a058bb2c 100644 --- a/include/hw/pci/pci_bridge.h +++ b/include/hw/pci/pci_bridge.h @@ -92,6 +92,7 @@ struct PXBDev { uint8_t bus_nr; uint16_t numa_node; bool bypass_iommu; + bool hdm_for_passthrough; struct cxl_dev { CXLHost *cxl_host_bridge; /* Pointer to a CXLHost */ } cxl; -- 2.37.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH 2/2] hw/pxb-cxl: Support passthrough HDM Decoders unless overridden @ 2023-01-23 12:17 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron via @ 2023-01-23 12:17 UTC (permalink / raw) To: qemu-devel Cc: Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Fan Ni The CXL r3.0 specification allows for there to be no HDM decoders on CXL Host Bridges if they have only a single root port. Instead, all accesses directed to the host bridge (as specified in CXL Fixed Memory Windows) are assumed to be routed to the single root port. Linux currently assumes this implementation choice. So to simplify testing, make QEMU emulation also default to no HDM decoders under these particular circumstances, but provide a hdm_for_passthrough boolean option to have HDM decoders as previously. Technically this is breaking backwards compatibility, but given the only known software stack used with the QEMU emulation is the Linux kernel and this configuration did not work before this change, there are unlikely to be any complaints that it now works. The option is retained to allow testing of software that does allow for these HDM decoders to exist, once someone writes it. Reported-by: Fan Ni <fan.ni@samsung.com> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- hw/cxl/cxl-host.c | 31 +++++++++++++-------- hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- include/hw/cxl/cxl.h | 1 + include/hw/cxl/cxl_component.h | 1 + include/hw/pci/pci_bridge.h | 1 + 5 files changed, 60 insertions(+), 17 deletions(-) diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c index 3c1ec8732a..6e923ceeaf 100644 --- a/hw/cxl/cxl-host.c +++ b/hw/cxl/cxl-host.c @@ -146,21 +146,28 @@ static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow *fw, hwaddr addr) return NULL; } - hb_cstate = cxl_get_hb_cstate(hb); - if (!hb_cstate) { - return NULL; - } + if (cxl_get_hb_passthrough(hb)) { + rp = pcie_find_port_first(hb->bus); + if (!rp) { + return NULL; + } + } else { + hb_cstate = cxl_get_hb_cstate(hb); + if (!hb_cstate) { + return NULL; + } - cache_mem = hb_cstate->crb.cache_mem_registers; + cache_mem = hb_cstate->crb.cache_mem_registers; - target_found = cxl_hdm_find_target(cache_mem, addr, &target); - if (!target_found) { - return NULL; - } + target_found = cxl_hdm_find_target(cache_mem, addr, &target); + if (!target_found) { + return NULL; + } - rp = pcie_find_port_by_pn(hb->bus, target); - if (!rp) { - return NULL; + rp = pcie_find_port_by_pn(hb->bus, target); + if (!rp) { + return NULL; + } } d = pci_bridge_get_sec_bus(PCI_BRIDGE(rp))->devices[0]; diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index e752a21292..e2aabc12aa 100644 --- a/hw/pci-bridge/pci_expander_bridge.c +++ b/hw/pci-bridge/pci_expander_bridge.c @@ -15,6 +15,7 @@ #include "hw/pci/pci.h" #include "hw/pci/pci_bus.h" #include "hw/pci/pci_host.h" +#include "hw/pci/pcie_port.h" #include "hw/qdev-properties.h" #include "hw/pci/pci_bridge.h" #include "hw/pci-bridge/pci_expander_bridge.h" @@ -79,6 +80,13 @@ CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb) return &host->cxl_cstate; } +bool cxl_get_hb_passthrough(PCIHostState *hb) +{ + CXLHost *host = PXB_CXL_HOST(hb); + + return host->passthrough; +} + static int pxb_bus_num(PCIBus *bus) { PXBDev *pxb = convert_to_pxb(bus->parent_dev); @@ -289,15 +297,31 @@ static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin) return pin - PCI_SLOT(pxb->devfn); } -static void pxb_dev_reset(DeviceState *dev) +static void pxb_cxl_dev_reset(DeviceState *dev) { CXLHost *cxl = PXB_CXL_DEV(dev)->cxl.cxl_host_bridge; CXLComponentState *cxl_cstate = &cxl->cxl_cstate; + PCIHostState *hb = PCI_HOST_BRIDGE(cxl); uint32_t *reg_state = cxl_cstate->crb.cache_mem_registers; uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask; + int dsp_count = 0; cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT); - ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT, 8); + /* + * The CXL specification allows for host bridges with no HDM decoders + * if they only have a single root port. + */ + if (!PXB_DEV(dev)->hdm_for_passthrough) { + dsp_count = pcie_count_ds_ports(hb->bus); + } + /* Initial reset will have 0 dsp so wait until > 0 */ + if (dsp_count == 1) { + cxl->passthrough = true; + /* Set Capability ID in header to NONE */ + ARRAY_FIELD_DP32(reg_state, CXL_HDM_CAPABILITY_HEADER, ID, 0); + } else { + ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT, 8); + } } static gint pxb_compare(gconstpointer a, gconstpointer b) @@ -481,9 +505,18 @@ static void pxb_cxl_dev_realize(PCIDevice *dev, Error **errp) } pxb_dev_realize_common(dev, CXL, errp); - pxb_dev_reset(DEVICE(dev)); + pxb_cxl_dev_reset(DEVICE(dev)); } +static Property pxb_cxl_dev_properties[] = { + /* Note: 0 is not a legal PXB bus number. */ + DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0), + DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, NUMA_NODE_UNASSIGNED), + DEFINE_PROP_BOOL("bypass_iommu", PXBDev, bypass_iommu, false), + DEFINE_PROP_BOOL("hdm_for_passthrough", PXBDev, hdm_for_passthrough, false), + DEFINE_PROP_END_OF_LIST(), +}; + static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -497,12 +530,12 @@ static void pxb_cxl_dev_class_init(ObjectClass *klass, void *data) */ dc->desc = "CXL Host Bridge"; - device_class_set_props(dc, pxb_dev_properties); + device_class_set_props(dc, pxb_cxl_dev_properties); set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); /* Host bridges aren't hotpluggable. FIXME: spec reference */ dc->hotpluggable = false; - dc->reset = pxb_dev_reset; + dc->reset = pxb_cxl_dev_reset; } static const TypeInfo pxb_cxl_dev_info = { diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h index 716f0f0f2a..9c8185ae88 100644 --- a/include/hw/cxl/cxl.h +++ b/include/hw/cxl/cxl.h @@ -50,6 +50,7 @@ struct CXLHost { PCIHostState parent_obj; CXLComponentState cxl_cstate; + bool passthrough; }; #define TYPE_PXB_CXL_HOST "pxb-cxl-host" diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h index 870faeb6dd..97e45e6747 100644 --- a/include/hw/cxl/cxl_component.h +++ b/include/hw/cxl/cxl_component.h @@ -251,6 +251,7 @@ static inline hwaddr cxl_decode_ig(int ig) } CXLComponentState *cxl_get_hb_cstate(PCIHostState *hb); +bool cxl_get_hb_passthrough(PCIHostState *hb); void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp); void cxl_doe_cdat_release(CXLComponentState *cxl_cstate); diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h index 63a7521567..81a058bb2c 100644 --- a/include/hw/pci/pci_bridge.h +++ b/include/hw/pci/pci_bridge.h @@ -92,6 +92,7 @@ struct PXBDev { uint8_t bus_nr; uint16_t numa_node; bool bypass_iommu; + bool hdm_for_passthrough; struct cxl_dev { CXLHost *cxl_host_bridge; /* Pointer to a CXLHost */ } cxl; -- 2.37.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <CGME20230123175325uscas1p134d834ae3636c7c56e93299c01a4f351@uscas1p1.samsung.com>]
* Re: [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation [not found] ` <CGME20230123175325uscas1p134d834ae3636c7c56e93299c01a4f351@uscas1p1.samsung.com> @ 2023-01-23 17:53 ` Fan Ni 2023-01-24 9:47 ` Jonathan Cameron via 0 siblings, 1 reply; 10+ messages in thread From: Fan Ni @ 2023-01-23 17:53 UTC (permalink / raw) To: Jonathan Cameron Cc: qemu-devel, Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Adam Manzanares, dave On Mon, Jan 23, 2023 at 12:17:10PM +0000, Jonathan Cameron wrote: > Until now, testing using CXL has relied up always using two root ports > below a host bridge, to work around a current assumption in the Linux > kernel support that, in the single root port case, the implementation will > use the allowed passthrough decoder implementation choice. If that choice > is made all accesses are routed from the host bridge to the single > root port that is present. Effectively we have a pass through decoder > (it is called that in the kernel driver). > > This patch series implements that functionality and makes it the default > See patch 2 for a discussion of why I think we can make this change > without backwards compatibility issues (basically if it didn't work before > who are we breaking by making it work?) > > Whilst this limitation has been known since the initial QEMU patch > postings / kernel CXL region support, Fan Ni Ran into it recently reminding > me that we should solve it. > > https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/ > > Tree with a large set of patches before this at: > https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20 > > I've done some basic testing, though I did hit what appears to be > a kernel race on region bring up of existing region / namespace in a > 1HB 2RP 2EP test case. That is proving hard to replicate consistently > but doesn't seem to have anything to do with the emulation other than > perhaps we are opening up a race by responding slowly to something. > > Jonathan Cameron (2): > hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers > hw/pxb-cxl: Support passthrough HDM Decoders unless overridden > > hw/cxl/cxl-host.c | 31 +++++++++++++-------- > hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- > hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ > include/hw/cxl/cxl.h | 1 + > include/hw/cxl/cxl_component.h | 1 + > include/hw/pci/pci_bridge.h | 1 + > include/hw/pci/pcie_port.h | 2 ++ > 7 files changed, 100 insertions(+), 17 deletions(-) > > -- > 2.37.2 > > Tried three different cxl topology setups (1HB1RP, 1HB2RP2Memdev, with switch), the patch works fine for me. Btw, there seem some format issues with the patch, got warnings with checkpatch tool. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation 2023-01-23 17:53 ` [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation Fan Ni @ 2023-01-24 9:47 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2023-01-24 9:47 UTC (permalink / raw) To: Fan Ni Cc: qemu-devel, Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Adam Manzanares, dave On Mon, 23 Jan 2023 17:53:24 +0000 Fan Ni <fan.ni@samsung.com> wrote: > On Mon, Jan 23, 2023 at 12:17:10PM +0000, Jonathan Cameron wrote: > > > > > Until now, testing using CXL has relied up always using two root ports > > below a host bridge, to work around a current assumption in the Linux > > kernel support that, in the single root port case, the implementation will > > use the allowed passthrough decoder implementation choice. If that choice > > is made all accesses are routed from the host bridge to the single > > root port that is present. Effectively we have a pass through decoder > > (it is called that in the kernel driver). > > > > This patch series implements that functionality and makes it the default > > See patch 2 for a discussion of why I think we can make this change > > without backwards compatibility issues (basically if it didn't work before > > who are we breaking by making it work?) > > > > Whilst this limitation has been known since the initial QEMU patch > > postings / kernel CXL region support, Fan Ni Ran into it recently reminding > > me that we should solve it. > > > > https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/ > > > > Tree with a large set of patches before this at: > > https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20 > > > > I've done some basic testing, though I did hit what appears to be > > a kernel race on region bring up of existing region / namespace in a > > 1HB 2RP 2EP test case. That is proving hard to replicate consistently > > but doesn't seem to have anything to do with the emulation other than > > perhaps we are opening up a race by responding slowly to something. > > > > Jonathan Cameron (2): > > hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers > > hw/pxb-cxl: Support passthrough HDM Decoders unless overridden > > > > hw/cxl/cxl-host.c | 31 +++++++++++++-------- > > hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- > > hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ > > include/hw/cxl/cxl.h | 1 + > > include/hw/cxl/cxl_component.h | 1 + > > include/hw/pci/pci_bridge.h | 1 + > > include/hw/pci/pcie_port.h | 2 ++ > > 7 files changed, 100 insertions(+), 17 deletions(-) > > > > -- > > 2.37.2 > > > > > Tried three different cxl topology setups (1HB1RP, 1HB2RP2Memdev, with > switch), the patch works fine for me. > Btw, there seem some format issues with the patch, got warnings with > checkpatch tool. Thanks! I'll clean those up. Was being lazy on it as it's an RFC for now :) Given this is small and useful I'll probably pull it nearer the head of the queue. When I repost, if you could give a Tested-by tag that would be great! Thanks, Jonathan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation @ 2023-01-24 9:47 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron via @ 2023-01-24 9:47 UTC (permalink / raw) To: Fan Ni Cc: qemu-devel, Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Adam Manzanares, dave On Mon, 23 Jan 2023 17:53:24 +0000 Fan Ni <fan.ni@samsung.com> wrote: > On Mon, Jan 23, 2023 at 12:17:10PM +0000, Jonathan Cameron wrote: > > > > > Until now, testing using CXL has relied up always using two root ports > > below a host bridge, to work around a current assumption in the Linux > > kernel support that, in the single root port case, the implementation will > > use the allowed passthrough decoder implementation choice. If that choice > > is made all accesses are routed from the host bridge to the single > > root port that is present. Effectively we have a pass through decoder > > (it is called that in the kernel driver). > > > > This patch series implements that functionality and makes it the default > > See patch 2 for a discussion of why I think we can make this change > > without backwards compatibility issues (basically if it didn't work before > > who are we breaking by making it work?) > > > > Whilst this limitation has been known since the initial QEMU patch > > postings / kernel CXL region support, Fan Ni Ran into it recently reminding > > me that we should solve it. > > > > https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/ > > > > Tree with a large set of patches before this at: > > https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20 > > > > I've done some basic testing, though I did hit what appears to be > > a kernel race on region bring up of existing region / namespace in a > > 1HB 2RP 2EP test case. That is proving hard to replicate consistently > > but doesn't seem to have anything to do with the emulation other than > > perhaps we are opening up a race by responding slowly to something. > > > > Jonathan Cameron (2): > > hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers > > hw/pxb-cxl: Support passthrough HDM Decoders unless overridden > > > > hw/cxl/cxl-host.c | 31 +++++++++++++-------- > > hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- > > hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ > > include/hw/cxl/cxl.h | 1 + > > include/hw/cxl/cxl_component.h | 1 + > > include/hw/pci/pci_bridge.h | 1 + > > include/hw/pci/pcie_port.h | 2 ++ > > 7 files changed, 100 insertions(+), 17 deletions(-) > > > > -- > > 2.37.2 > > > > > Tried three different cxl topology setups (1HB1RP, 1HB2RP2Memdev, with > switch), the patch works fine for me. > Btw, there seem some format issues with the patch, got warnings with > checkpatch tool. Thanks! I'll clean those up. Was being lazy on it as it's an RFC for now :) Given this is small and useful I'll probably pull it nearer the head of the queue. When I repost, if you could give a Tested-by tag that would be great! Thanks, Jonathan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation 2023-01-24 9:47 ` Jonathan Cameron via (?) @ 2023-01-24 17:00 ` Fan Ni -1 siblings, 0 replies; 10+ messages in thread From: Fan Ni @ 2023-01-24 17:00 UTC (permalink / raw) To: Jonathan Cameron Cc: qemu-devel, Ben Widawsky, linux-cxl, linuxarm, Ira Weiny, Dave Jiang, alison.schofield, Adam Manzanares, dave On Tue, Jan 24, 2023 at 09:47:20AM +0000, Jonathan Cameron wrote: > On Mon, 23 Jan 2023 17:53:24 +0000 > Fan Ni <fan.ni@samsung.com> wrote: > > > On Mon, Jan 23, 2023 at 12:17:10PM +0000, Jonathan Cameron wrote: > > > > > > > > > Until now, testing using CXL has relied up always using two root ports > > > below a host bridge, to work around a current assumption in the Linux > > > kernel support that, in the single root port case, the implementation will > > > use the allowed passthrough decoder implementation choice. If that choice > > > is made all accesses are routed from the host bridge to the single > > > root port that is present. Effectively we have a pass through decoder > > > (it is called that in the kernel driver). > > > > > > This patch series implements that functionality and makes it the default > > > See patch 2 for a discussion of why I think we can make this change > > > without backwards compatibility issues (basically if it didn't work before > > > who are we breaking by making it work?) > > > > > > Whilst this limitation has been known since the initial QEMU patch > > > postings / kernel CXL region support, Fan Ni Ran into it recently reminding > > > me that we should solve it. > > > > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-cxl/20230113171044.GA24788@bgt-140510-bm03/__;!!EwVzqGoTKBqv-0DWAJBm!WsD6FV-KV_YnhhHWLll72cHLqLQ_8kpps3MlpAa6Bonsdz6aifuWT40-QnlRyFqWyeyaVb-RiC03_qajbeCGsI5DcPYv$ > > > > > > Tree with a large set of patches before this at: > > > https://urldefense.com/v3/__https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-20__;!!EwVzqGoTKBqv-0DWAJBm!WsD6FV-KV_YnhhHWLll72cHLqLQ_8kpps3MlpAa6Bonsdz6aifuWT40-QnlRyFqWyeyaVb-RiC03_qajbeCGsPjbv12T$ > > > > > > I've done some basic testing, though I did hit what appears to be > > > a kernel race on region bring up of existing region / namespace in a > > > 1HB 2RP 2EP test case. That is proving hard to replicate consistently > > > but doesn't seem to have anything to do with the emulation other than > > > perhaps we are opening up a race by responding slowly to something. > > > > > > Jonathan Cameron (2): > > > hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers > > > hw/pxb-cxl: Support passthrough HDM Decoders unless overridden > > > > > > hw/cxl/cxl-host.c | 31 +++++++++++++-------- > > > hw/pci-bridge/pci_expander_bridge.c | 43 +++++++++++++++++++++++++---- > > > hw/pci/pcie_port.c | 38 +++++++++++++++++++++++++ > > > include/hw/cxl/cxl.h | 1 + > > > include/hw/cxl/cxl_component.h | 1 + > > > include/hw/pci/pci_bridge.h | 1 + > > > include/hw/pci/pcie_port.h | 2 ++ > > > 7 files changed, 100 insertions(+), 17 deletions(-) > > > > > > -- > > > 2.37.2 > > > > > > > > Tried three different cxl topology setups (1HB1RP, 1HB2RP2Memdev, with > > switch), the patch works fine for me. > > Btw, there seem some format issues with the patch, got warnings with > > checkpatch tool. > Thanks! I'll clean those up. Was being lazy on it as it's an RFC for > now :) Given this is small and useful I'll probably pull it nearer the > head of the queue. > > When I repost, if you could give a Tested-by tag that would be great! > > Thanks, > > Jonathan > Will do. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-01-24 17:00 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-23 12:17 [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation Jonathan Cameron
2023-01-23 12:17 ` Jonathan Cameron via
2023-01-23 12:17 ` [RFC PATCH 1/2] hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers Jonathan Cameron
2023-01-23 12:17 ` Jonathan Cameron via
2023-01-23 12:17 ` [RFC PATCH 2/2] hw/pxb-cxl: Support passthrough HDM Decoders unless overridden Jonathan Cameron
2023-01-23 12:17 ` Jonathan Cameron via
[not found] ` <CGME20230123175325uscas1p134d834ae3636c7c56e93299c01a4f351@uscas1p1.samsung.com>
2023-01-23 17:53 ` [RFC PATCH 0/2] hw/cxl: Passthrough HDM decoder emulation Fan Ni
2023-01-24 9:47 ` Jonathan Cameron
2023-01-24 9:47 ` Jonathan Cameron via
2023-01-24 17:00 ` Fan Ni
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.