* [RFC PATCH v2 0/2] cxl: Support creation of a new CXL Host Bridge
@ 2025-05-30 10:33 wangyuquan
2025-05-30 10:33 ` [RFC PATCH v2 1/2] hw/pxb-cxl: Rename the pxb cxl host bridge wangyuquan
2025-05-30 10:33 ` [RFC PATCH v2 2/2] pci-host/cxl: Support creation of a new CXL Host Bridge wangyuquan
0 siblings, 2 replies; 5+ messages in thread
From: wangyuquan @ 2025-05-30 10:33 UTC (permalink / raw)
To: jonathan.cameron, fan.ni, mst, marcel.apfelbaum
Cc: chenbaozi, qemu-devel, linux-cxl, wangyuquan
v1 -> v2:
- Move the code of new bridge to hw/pci-host/cxl.c
- Fix and simplify some logic on handling the different bridge types
Background
==========
Currently the base CXL support for arm platforms is only on Jonathan's
patches[1]. Some platform like SBSA-REF can be more like a real machine,
thus the support of cxl could be meaningful. However, the pxb-cxl-host
realization on this platform seems not satisfying their requirements[2].
New CXL HOST design
===================
This work defines a new cxl host bridge type (TYPE_CXL_HOST). This
could be considered as a prototype of an independent cxl host bridge
which combines gpex features (ecam, mmio windows & irq) and CXL features.
The root bus path of CXL_HOST is "0001:00", that would not affect the
original pcie host topology. In the previous, the pxb-cxl-host with
any cxl root ports and cxl endpoint devices would occupy the BDF
number of the original pcie domain. This new type provide a solution
to resolve the problem.
Remaining problems
==================
I tried to use 'object_resolve_path' but it could not work in
'cxl_fmws_link_targets', so I used 'TYPE_DEVICE' to match that.
Yuquan Wang (2):
hw/pxb-cxl: Rename the pxb cxl host bridge
pci-host/cxl: Support creation of a new CXL Host Bridge
hw/cxl/cxl-host-stubs.c | 2 +
hw/cxl/cxl-host.c | 66 +++++++++---
hw/pci-bridge/pci_expander_bridge.c | 8 +-
hw/pci-host/cxl.c | 145 ++++++++++++++++++++++++++
hw/pci-host/meson.build | 1 +
include/hw/cxl/cxl.h | 7 +-
include/hw/cxl/cxl_host.h | 2 +
include/hw/pci-host/cxl_host_bridge.h | 25 +++++
8 files changed, 234 insertions(+), 22 deletions(-)
create mode 100644 hw/pci-host/cxl.c
create mode 100644 include/hw/pci-host/cxl_host_bridge.h
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH v2 1/2] hw/pxb-cxl: Rename the pxb cxl host bridge
2025-05-30 10:33 [RFC PATCH v2 0/2] cxl: Support creation of a new CXL Host Bridge wangyuquan
@ 2025-05-30 10:33 ` wangyuquan
2025-06-12 14:37 ` Jonathan Cameron
2025-05-30 10:33 ` [RFC PATCH v2 2/2] pci-host/cxl: Support creation of a new CXL Host Bridge wangyuquan
1 sibling, 1 reply; 5+ messages in thread
From: wangyuquan @ 2025-05-30 10:33 UTC (permalink / raw)
To: jonathan.cameron, fan.ni, mst, marcel.apfelbaum
Cc: chenbaozi, qemu-devel, linux-cxl, wangyuquan
This renames some descriptions and definitions of pxb cxl host
bridge, since the original names can be confusing.
Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
---
hw/pci-bridge/pci_expander_bridge.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 3a29dfefc2..bd637786d0 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -201,7 +201,7 @@ static void pxb_cxl_realize(DeviceState *dev, Error **errp)
}
/*
- * Host bridge realization has no means of knowning state associated
+ * PXB Host bridge realization has no means of knowning state associated
* with a particular machine. As such, it is nececssary to delay
* final setup of the host bridge register space until later in the
* machine bring up.
@@ -240,7 +240,7 @@ static void pxb_cxl_host_class_init(ObjectClass *class, const void *data)
* This is a device to handle the MMIO for a CXL host bridge. It does nothing
* else.
*/
-static const TypeInfo cxl_host_info = {
+static const TypeInfo pxb_cxl_host_info = {
.name = TYPE_PXB_CXL_HOST,
.parent = TYPE_PCI_HOST_BRIDGE,
.instance_size = sizeof(CXLHost),
@@ -522,7 +522,7 @@ static void pxb_cxl_dev_class_init(ObjectClass *klass, const void *data)
* vendor, device, class, etc. ids are intentionally left out.
*/
- dc->desc = "CXL Host Bridge";
+ dc->desc = "PXB CXL Host Bridge";
device_class_set_props(dc, pxb_cxl_dev_properties);
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
@@ -549,7 +549,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_cxl_host_info);
type_register_static(&pxb_dev_info);
type_register_static(&pxb_pcie_dev_info);
type_register_static(&pxb_cxl_dev_info);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC PATCH v2 2/2] pci-host/cxl: Support creation of a new CXL Host Bridge
2025-05-30 10:33 [RFC PATCH v2 0/2] cxl: Support creation of a new CXL Host Bridge wangyuquan
2025-05-30 10:33 ` [RFC PATCH v2 1/2] hw/pxb-cxl: Rename the pxb cxl host bridge wangyuquan
@ 2025-05-30 10:33 ` wangyuquan
2025-06-12 15:01 ` Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: wangyuquan @ 2025-05-30 10:33 UTC (permalink / raw)
To: jonathan.cameron, fan.ni, mst, marcel.apfelbaum
Cc: chenbaozi, qemu-devel, linux-cxl, Yuquan Wang
From: Yuquan Wang <wangyuquan1236@phytium.com.cn>
This work defines a new cxl host bridge type (TYPE_CXL_HOST). This
could be considered as a prototype of an independent cxl host bridge
which combines gpex features (ecam, mmio windows & irq) and CHBCR
at meanwhile.
The root bus path of CXL_HOST is "0001:00", that would not affect the
original pcie host topology. In the previous, the pxb-cxl-host with
any cxl root ports and cxl endpoint devices would occupy the BDF
number of the original pcie domain. This new type provide a solution
to resolve the problem.
CXLFixedWindow struct adds a new member 'target_chb' to record the
target list of CXLHostBridge and adjusts the logic of
'cxl_cfmws_find_device' and 'cxl_fmws_link_targets' to allow different
types of cxl host bridge.
Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
---
hw/cxl/cxl-host-stubs.c | 2 +
hw/cxl/cxl-host.c | 66 +++++++++---
hw/pci-host/cxl.c | 145 ++++++++++++++++++++++++++
hw/pci-host/meson.build | 1 +
include/hw/cxl/cxl.h | 7 +-
include/hw/cxl/cxl_host.h | 2 +
include/hw/pci-host/cxl_host_bridge.h | 25 +++++
7 files changed, 230 insertions(+), 18 deletions(-)
create mode 100644 hw/pci-host/cxl.c
create mode 100644 include/hw/pci-host/cxl_host_bridge.h
diff --git a/hw/cxl/cxl-host-stubs.c b/hw/cxl/cxl-host-stubs.c
index cae4afcdde..aea94933ba 100644
--- a/hw/cxl/cxl-host-stubs.c
+++ b/hw/cxl/cxl-host-stubs.c
@@ -11,5 +11,7 @@
void cxl_fmws_link_targets(CXLState *stat, Error **errp) {};
void cxl_machine_init(Object *obj, CXLState *state) {};
void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp) {};
+void cxl_fixed_memory_window_config(CXLState *cxl_state,
+ CXLFixedMemoryWindowOptions *object, Error **errp) {};
const MemoryRegionOps cfmws_ops;
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index e010163174..183bc19a4b 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -16,15 +16,17 @@
#include "qapi/qapi-visit-machine.h"
#include "hw/cxl/cxl.h"
#include "hw/cxl/cxl_host.h"
+#include "hw/irq.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_host.h"
#include "hw/pci/pcie_port.h"
#include "hw/pci-bridge/pci_expander_bridge.h"
+#include "hw/pci-host/cxl_host_bridge.h"
-static void cxl_fixed_memory_window_config(CXLState *cxl_state,
- CXLFixedMemoryWindowOptions *object,
- Error **errp)
+void cxl_fixed_memory_window_config(CXLState *cxl_state,
+ CXLFixedMemoryWindowOptions *object,
+ Error **errp)
{
ERRP_GUARD();
g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
@@ -83,14 +85,16 @@ void cxl_fmws_link_targets(CXLState *cxl_state, Error **errp)
bool ambig;
o = object_resolve_path_type(fw->targets[i],
- TYPE_PXB_CXL_DEV,
- &ambig);
- if (!o) {
+ TYPE_DEVICE, &ambig);
+
+ if (object_dynamic_cast(o, TYPE_PXB_CXL_DEV) ||
+ object_dynamic_cast(o, TYPE_CXL_HOST)) {
+ fw->target_hbs[i] = o;
+ } else {
error_setg(errp, "Could not resolve CXLFM target %s",
- fw->targets[i]);
+ fw->targets[i]);
return;
}
- fw->target_hbs[i] = PXB_CXL_DEV(o);
}
}
}
@@ -153,6 +157,7 @@ static bool cxl_hdm_find_target(uint32_t *cache_mem, hwaddr addr,
static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow *fw, hwaddr addr)
{
CXLComponentState *hb_cstate, *usp_cstate;
+ CXLHostBridge *cxlhost;
PCIHostState *hb;
CXLUpstreamPort *usp;
int rb_index;
@@ -160,23 +165,50 @@ static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow *fw, hwaddr addr)
uint8_t target;
bool target_found;
PCIDevice *rp, *d;
+ Object *o;
/* Address is relative to memory region. Convert to HPA */
addr += fw->base;
rb_index = (addr / cxl_decode_ig(fw->enc_int_gran)) % fw->num_targets;
- hb = PCI_HOST_BRIDGE(fw->target_hbs[rb_index]->cxl_host_bridge);
- if (!hb || !hb->bus || !pci_bus_is_cxl(hb->bus)) {
- return NULL;
- }
-
- if (cxl_get_hb_passthrough(hb)) {
- rp = pcie_find_port_first(hb->bus);
- if (!rp) {
+ o = fw->target_hbs[rb_index];
+ if (object_dynamic_cast(o, TYPE_PXB_CXL_DEV)) {
+ hb = PCI_HOST_BRIDGE(PXB_CXL_DEV(o)->cxl_host_bridge);
+ if (!hb || !hb->bus || !pci_bus_is_cxl(hb->bus)) {
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;
+
+ 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;
+ }
+ }
} else {
- hb_cstate = cxl_get_hb_cstate(hb);
+ hb = PCI_HOST_BRIDGE(o);
+ if (!hb || !hb->bus || !pci_bus_is_cxl(hb->bus)) {
+ return NULL;
+ }
+
+ cxlhost = CXL_HOST(hb);
+ hb_cstate = &cxlhost->cxl_cstate;
if (!hb_cstate) {
return NULL;
}
diff --git a/hw/pci-host/cxl.c b/hw/pci-host/cxl.c
new file mode 100644
index 0000000000..8323456864
--- /dev/null
+++ b/hw/pci-host/cxl.c
@@ -0,0 +1,145 @@
+#include "qemu/osdep.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci-host/cxl_host_bridge.h"
+
+static void cxl_host_set_irq(void *opaque, int irq_num, int level)
+{
+ CXLHostBridge *host = opaque;
+
+ qemu_set_irq(host->irq[irq_num], level);
+}
+
+int cxl_host_set_irq_num(CXLHostBridge *host, int index, int gsi)
+{
+ if (index >= CXL_HOST_NUM_IRQS) {
+ return -EINVAL;
+ }
+
+ host->irq_num[index] = gsi;
+ return 0;
+}
+
+static PCIINTxRoute cxl_host_route_intx_pin_to_irq(void *opaque, int pin)
+{
+ PCIINTxRoute route;
+ CXLHostBridge *host = opaque;
+ int gsi = host->irq_num[pin];
+
+ route.irq = gsi;
+ if (gsi < 0) {
+ route.mode = PCI_INTX_DISABLED;
+ } else {
+ route.mode = PCI_INTX_ENABLED;
+ }
+
+ return route;
+}
+
+static const char *cxl_host_root_bus_path(PCIHostState *host_bridge,
+ PCIBus *rootbus)
+{
+ return "0001:00";
+}
+
+void cxl_host_hook_up_registers(CXLState *cxl_state, CXLHostBridge *host)
+{
+ CXLComponentState *cxl_cstate = &host->cxl_cstate;
+ struct MemoryRegion *mr = &cxl_cstate->crb.component_registers;
+
+ memory_region_add_subregion(&cxl_state->host_mr, 0, mr);
+}
+
+static void cxl_host_reset(CXLHostBridge *host)
+{
+ CXLComponentState *cxl_cstate = &host->cxl_cstate;
+ uint32_t *reg_state = cxl_cstate->crb.cache_mem_registers;
+ uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask;
+
+ cxl_component_register_init_common(reg_state, write_msk, CXL2_RC);
+
+ ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT,
+ 8);
+}
+
+static void cxl_host_realize(DeviceState *dev, Error **errp)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ CXLHostBridge *host = CXL_HOST(dev);
+ CXLComponentState *cxl_cstate = &host->cxl_cstate;
+ struct MemoryRegion *mr = &cxl_cstate->crb.component_registers;
+ PCIBus *cxlbus;
+ PCIHostState *pci = PCI_HOST_BRIDGE(dev);
+ PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
+ int i;
+
+ /* CHBCR MMIO init */
+ cxl_host_reset(host);
+ cxl_component_register_block_init(OBJECT(dev), cxl_cstate, TYPE_CXL_HOST);
+ sysbus_init_mmio(sbd, mr);
+
+ /* MMFG window init */
+ pcie_host_mmcfg_init(pex, PCIE_MMCFG_SIZE_MAX);
+ sysbus_init_mmio(sbd, &pex->mmio);
+
+ /* mmio window init */
+ memory_region_init(&host->io_mmio, OBJECT(host), "cxl_host_mmio",
+ UINT64_MAX);
+
+ memory_region_init_io(&host->io_mmio_window, OBJECT(host),
+ &unassigned_io_ops, OBJECT(host),
+ "cxl_host_mmio_window", UINT64_MAX);
+
+ memory_region_add_subregion(&host->io_mmio_window, 0, &host->io_mmio);
+ sysbus_init_mmio(sbd, &host->io_mmio_window);
+
+ /* ioport window init, 64K is the legacy size in x86 */
+ memory_region_init(&host->io_ioport, OBJECT(host), "cxl_host_ioport",
+ 64 * 1024);
+
+ memory_region_init_io(&host->io_ioport_window, OBJECT(host),
+ &unassigned_io_ops, OBJECT(host),
+ "cxl_host_ioport_window", 64 * 1024);
+
+ memory_region_add_subregion(&host->io_ioport_window, 0, &host->io_ioport);
+ sysbus_init_mmio(sbd, &host->io_ioport_window);
+
+ /* PCIe host bridge use 4 legacy IRQ lines */
+ for (i = 0; i < CXL_HOST_NUM_IRQS; i++) {
+ sysbus_init_irq(sbd, &host->irq[i]);
+ host->irq_num[i] = -1;
+ }
+
+ pci->bus = pci_register_root_bus(dev, "cxlhost.0", cxl_host_set_irq,
+ pci_swizzle_map_irq_fn, host, &host->io_mmio,
+ &host->io_ioport, 0, 4, TYPE_CXL_BUS);
+ cxlbus = pci->bus;
+ cxlbus->flags |= PCI_BUS_CXL;
+
+ pci_bus_set_route_irq_fn(pci->bus, cxl_host_route_intx_pin_to_irq);
+}
+
+static void cxl_host_class_init(ObjectClass *class, const void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(class);
+ PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
+
+ hc->root_bus_path = cxl_host_root_bus_path;
+ dc->realize = cxl_host_realize;
+ dc->desc = "CXL Host Bridge";
+ set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+ dc->fw_name = "cxl";
+}
+
+static const TypeInfo cxl_host_info = {
+ .name = TYPE_CXL_HOST,
+ .parent = TYPE_PCIE_HOST_BRIDGE,
+ .instance_size = sizeof(CXLHostBridge),
+ .class_init = cxl_host_class_init,
+};
+
+static void cxl_host_register(void)
+{
+ type_register_static(&cxl_host_info);
+}
+
+type_init(cxl_host_register)
diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build
index 937a0f72ac..a00995068b 100644
--- a/hw/pci-host/meson.build
+++ b/hw/pci-host/meson.build
@@ -4,6 +4,7 @@ pci_ss.add(when: 'CONFIG_PCI_BONITO', if_true: files('bonito.c'))
pci_ss.add(when: 'CONFIG_GT64120', if_true: files('gt64120.c'))
pci_ss.add(when: 'CONFIG_PCI_EXPRESS_DESIGNWARE', if_true: files('designware.c'))
pci_ss.add(when: 'CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', if_true: files('gpex.c'))
+pci_ss.add(when: 'CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', if_true: files('cxl.c'))
pci_ss.add(when: ['CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', 'CONFIG_ACPI'], if_true: files('gpex-acpi.c'))
pci_ss.add(when: 'CONFIG_PCI_EXPRESS_Q35', if_true: files('q35.c'))
pci_ss.add(when: 'CONFIG_PCI_EXPRESS_XILINX', if_true: files('xilinx-pcie.c'))
diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 75e47b6864..346a7b58ad 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -26,10 +26,12 @@
typedef struct PXBCXLDev PXBCXLDev;
+typedef struct CXLHostBridge CXLHostBridge;
+
typedef struct CXLFixedWindow {
uint64_t size;
char **targets;
- PXBCXLDev *target_hbs[16];
+ Object *target_hbs[16];
uint8_t num_targets;
uint8_t enc_int_ways;
uint8_t enc_int_gran;
@@ -56,6 +58,9 @@ struct CXLHost {
#define TYPE_PXB_CXL_HOST "pxb-cxl-host"
OBJECT_DECLARE_SIMPLE_TYPE(CXLHost, PXB_CXL_HOST)
+#define TYPE_CXL_HOST "cxl-host"
+OBJECT_DECLARE_SIMPLE_TYPE(CXLHostBridge, CXL_HOST)
+
#define TYPE_CXL_USP "cxl-upstream"
typedef struct CXLUpstreamPort CXLUpstreamPort;
diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
index c9bc9c7c50..f3184733aa 100644
--- a/include/hw/cxl/cxl_host.h
+++ b/include/hw/cxl/cxl_host.h
@@ -16,6 +16,8 @@
void cxl_machine_init(Object *obj, CXLState *state);
void cxl_fmws_link_targets(CXLState *stat, Error **errp);
void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp);
+void cxl_fixed_memory_window_config(CXLState *cxl_state,
+ CXLFixedMemoryWindowOptions *object, Error **errp);
extern const MemoryRegionOps cfmws_ops;
diff --git a/include/hw/pci-host/cxl_host_bridge.h b/include/hw/pci-host/cxl_host_bridge.h
new file mode 100644
index 0000000000..f6830dab83
--- /dev/null
+++ b/include/hw/pci-host/cxl_host_bridge.h
@@ -0,0 +1,25 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "hw/cxl/cxl.h"
+#include "hw/irq.h"
+#include "hw/pci/pcie_host.h"
+
+#define CXL_HOST_NUM_IRQS 4
+
+typedef struct CXLHostBridge {
+ PCIExpressHost parent_obj;
+
+ CXLComponentState cxl_cstate;
+
+ MemoryRegion io_ioport;
+ MemoryRegion io_mmio;
+ MemoryRegion io_ioport_window;
+ MemoryRegion io_mmio_window;
+ qemu_irq irq[CXL_HOST_NUM_IRQS];
+ int irq_num[CXL_HOST_NUM_IRQS];
+} CXLHostBridge;
+
+int cxl_host_set_irq_num(CXLHostBridge *host, int index, int gsi);
+void cxl_host_hook_up_registers(CXLState *cxl_state, CXLHostBridge *host);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 1/2] hw/pxb-cxl: Rename the pxb cxl host bridge
2025-05-30 10:33 ` [RFC PATCH v2 1/2] hw/pxb-cxl: Rename the pxb cxl host bridge wangyuquan
@ 2025-06-12 14:37 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2025-06-12 14:37 UTC (permalink / raw)
To: wangyuquan
Cc: fan.ni, mst, marcel.apfelbaum, chenbaozi, qemu-devel, linux-cxl
On Fri, 30 May 2025 18:33:19 +0800
wangyuquan <wangyuquan1236@phytium.com.cn> wrote:
> This renames some descriptions and definitions of pxb cxl host
> bridge, since the original names can be confusing.
>
> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
Seems reasonable though only needed if we move forward with
patch 2.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 2/2] pci-host/cxl: Support creation of a new CXL Host Bridge
2025-05-30 10:33 ` [RFC PATCH v2 2/2] pci-host/cxl: Support creation of a new CXL Host Bridge wangyuquan
@ 2025-06-12 15:01 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2025-06-12 15:01 UTC (permalink / raw)
To: wangyuquan
Cc: fan.ni, mst, marcel.apfelbaum, chenbaozi, qemu-devel, linux-cxl
On Fri, 30 May 2025 18:33:20 +0800
wangyuquan <wangyuquan1236@phytium.com.cn> wrote:
> From: Yuquan Wang <wangyuquan1236@phytium.com.cn>
>
> This work defines a new cxl host bridge type (TYPE_CXL_HOST). This
I'd stick to simpler text.
Define a new CXL host bridge type (TYPE_CXL_HOST). This is an
independent CXL host bridge which combined GPEX features (ECAM, MMIO
windows and irq) and CXL Host Bridge Component Registers (CHBCR)
> could be considered as a prototype of an independent cxl host bridge
> which combines gpex features (ecam, mmio windows & irq) and CHBCR
> at meanwhile.
>
> The root bus path of CXL_HOST is "0001:00", that would not affect the
> original pcie host topology. In the previous, the pxb-cxl-host with
PCIe or PCIE
> any cxl root ports and cxl endpoint devices would occupy the BDF
CXL
> number of the original pcie domain. This new type provide a solution
> to resolve the problem.
This isn't describing a problem as such. I think the problem is with
the generated ACPI tables etc being wrong. Perhaps some more details?
>
> CXLFixedWindow struct adds a new member 'target_chb' to record the
> target list of CXLHostBridge and adjusts the logic of
> 'cxl_cfmws_find_device' and 'cxl_fmws_link_targets' to allow different
> types of cxl host bridge.
>
> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
A few comments inline. I don't mind this support being added as
long as we get the user in (so SBSA patch as well).
> diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> index e010163174..183bc19a4b 100644
> --- a/hw/cxl/cxl-host.c
> +++ b/hw/cxl/cxl-host.c
> -static void cxl_fixed_memory_window_config(CXLState *cxl_state,
> - CXLFixedMemoryWindowOptions *object,
> - Error **errp)
> +void cxl_fixed_memory_window_config(CXLState *cxl_state,
> + CXLFixedMemoryWindowOptions *object,
> + Error **errp)
> {
> ERRP_GUARD();
> g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
> @@ -83,14 +85,16 @@ void cxl_fmws_link_targets(CXLState *cxl_state, Error **errp)
> bool ambig;
>
> o = object_resolve_path_type(fw->targets[i],
> - TYPE_PXB_CXL_DEV,
> - &ambig);
> - if (!o) {
> + TYPE_DEVICE, &ambig);
Keep alignment as it was. If nothing else it makes it more obvious what
exactly is changing in this diff.
> +
> + if (object_dynamic_cast(o, TYPE_PXB_CXL_DEV) ||
> + object_dynamic_cast(o, TYPE_CXL_HOST)) {
> + fw->target_hbs[i] = o;
> + } else {
> error_setg(errp, "Could not resolve CXLFM target %s",
> - fw->targets[i]);
> + fw->targets[i]);
Check for unnecessary white space changes like this one as they make it
harder to read the patch.
> return;
> }
> - fw->target_hbs[i] = PXB_CXL_DEV(o);
> }
> }
> }
> diff --git a/hw/pci-host/cxl.c b/hw/pci-host/cxl.c
> new file mode 100644
> index 0000000000..8323456864
> --- /dev/null
> +++ b/hw/pci-host/cxl.c
> @@ -0,0 +1,145 @@
Needs SPDX header I think.
> +#include "qemu/osdep.h"
> +#include "hw/pci/pci_bus.h"
> +#include "hw/pci-host/cxl_host_bridge.h"
> +
> +static void cxl_host_realize(DeviceState *dev, Error **errp)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + CXLHostBridge *host = CXL_HOST(dev);
> + CXLComponentState *cxl_cstate = &host->cxl_cstate;
> + struct MemoryRegion *mr = &cxl_cstate->crb.component_registers;
> + PCIBus *cxlbus;
Ordering seems a little random. I'd move this cxlbus to after pex.
> + PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> + PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
> + int i;
> +
> + /* CHBCR MMIO init */
> + cxl_host_reset(host);
> + cxl_component_register_block_init(OBJECT(dev), cxl_cstate, TYPE_CXL_HOST);
> + sysbus_init_mmio(sbd, mr);
> +
> + /* MMFG window init */
MMCFG however, I'd just drop this comment as the code is fairly obvious. Also dro
the CHBCR one above for the same reason.
> + pcie_host_mmcfg_init(pex, PCIE_MMCFG_SIZE_MAX);
> + sysbus_init_mmio(sbd, &pex->mmio);
> +
> + /* mmio window init */
And this comment. None of them are adding significant value.
> + memory_region_init(&host->io_mmio, OBJECT(host), "cxl_host_mmio",
> + UINT64_MAX);
> +
> + memory_region_init_io(&host->io_mmio_window, OBJECT(host),
> + &unassigned_io_ops, OBJECT(host),
> + "cxl_host_mmio_window", UINT64_MAX);
> +
> + memory_region_add_subregion(&host->io_mmio_window, 0, &host->io_mmio);
> + sysbus_init_mmio(sbd, &host->io_mmio_window);
> +
> + /* ioport window init, 64K is the legacy size in x86 */
> + memory_region_init(&host->io_ioport, OBJECT(host), "cxl_host_ioport",
> + 64 * 1024);
> +
> + memory_region_init_io(&host->io_ioport_window, OBJECT(host),
> + &unassigned_io_ops, OBJECT(host),
> + "cxl_host_ioport_window", 64 * 1024);
> +
> + memory_region_add_subregion(&host->io_ioport_window, 0, &host->io_ioport);
> + sysbus_init_mmio(sbd, &host->io_ioport_window);
> +
> + /* PCIe host bridge use 4 legacy IRQ lines */
> + for (i = 0; i < CXL_HOST_NUM_IRQS; i++) {
> + sysbus_init_irq(sbd, &host->irq[i]);
> + host->irq_num[i] = -1;
> + }
> +
> + pci->bus = pci_register_root_bus(dev, "cxlhost.0", cxl_host_set_irq,
> + pci_swizzle_map_irq_fn, host, &host->io_mmio,
> + &host->io_ioport, 0, 4, TYPE_CXL_BUS);
> + cxlbus = pci->bus;
> + cxlbus->flags |= PCI_BUS_CXL;
> +
> + pci_bus_set_route_irq_fn(pci->bus, cxl_host_route_intx_pin_to_irq);
> +}
> diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build
> index 937a0f72ac..a00995068b 100644
> --- a/hw/pci-host/meson.build
> +++ b/hw/pci-host/meson.build
> @@ -4,6 +4,7 @@ pci_ss.add(when: 'CONFIG_PCI_BONITO', if_true: files('bonito.c'))
> pci_ss.add(when: 'CONFIG_GT64120', if_true: files('gt64120.c'))
> pci_ss.add(when: 'CONFIG_PCI_EXPRESS_DESIGNWARE', if_true: files('designware.c'))
> pci_ss.add(when: 'CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', if_true: files('gpex.c'))
> +pci_ss.add(when: 'CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', if_true: files('cxl.c'))
I think this needs it's own config variable.
> pci_ss.add(when: ['CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', 'CONFIG_ACPI'], if_true: files('gpex-acpi.c'))
> pci_ss.add(when: 'CONFIG_PCI_EXPRESS_Q35', if_true: files('q35.c'))
> pci_ss.add(when: 'CONFIG_PCI_EXPRESS_XILINX', if_true: files('xilinx-pcie.c'))
> diff --git a/include/hw/pci-host/cxl_host_bridge.h b/include/hw/pci-host/cxl_host_bridge.h
> new file mode 100644
> index 0000000000..f6830dab83
> --- /dev/null
> +++ b/include/hw/pci-host/cxl_host_bridge.h
> @@ -0,0 +1,25 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "hw/cxl/cxl.h"
> +#include "hw/irq.h"
> +#include "hw/pci/pcie_host.h"
> +
> +#define CXL_HOST_NUM_IRQS 4
Does that come directly from the PCIe bus spec, if so is there
a PCIe definition somewhere we can use for this?
None of the CXL spec defined stuff uses legacy interrupts
but I guess we have to support them for any PCIe device plugged
in to the bus.
> +
> +typedef struct CXLHostBridge {
> + PCIExpressHost parent_obj;
> +
> + CXLComponentState cxl_cstate;
> +
> + MemoryRegion io_ioport;
> + MemoryRegion io_mmio;
> + MemoryRegion io_ioport_window;
> + MemoryRegion io_mmio_window;
> + qemu_irq irq[CXL_HOST_NUM_IRQS];
> + int irq_num[CXL_HOST_NUM_IRQS];
> +} CXLHostBridge;
> +
> +int cxl_host_set_irq_num(CXLHostBridge *host, int index, int gsi);
> +void cxl_host_hook_up_registers(CXLState *cxl_state, CXLHostBridge *host);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-12 15:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-30 10:33 [RFC PATCH v2 0/2] cxl: Support creation of a new CXL Host Bridge wangyuquan
2025-05-30 10:33 ` [RFC PATCH v2 1/2] hw/pxb-cxl: Rename the pxb cxl host bridge wangyuquan
2025-06-12 14:37 ` Jonathan Cameron
2025-05-30 10:33 ` [RFC PATCH v2 2/2] pci-host/cxl: Support creation of a new CXL Host Bridge wangyuquan
2025-06-12 15:01 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).