* [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl
@ 2025-05-13 11:14 Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow Jonathan Cameron
` (5 more replies)
0 siblings, 6 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-13 11:14 UTC (permalink / raw)
To: qemu-devel, Fan Ni, Peter Maydell, mst
Cc: linux-cxl, linuxarm, qemu-arm, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
V13:
- Make CXL fixed memory windows sysbus devices.
IIRC this was requested by Peter in one of the reviews a long time back
but at the time the motivation was less strong than it becomes with some
WiP patches for hotness monitoring and high performance direct connect
where we need a machine type independent way to iterate all the CXL
fixed memory windows. This is a convenient place to do it so drag that
work forward into this series.
This allows us to drop separate list and necessary machine specific
access code in favour of
object_child_foreach_recursive(object_get_root(),...)
One snag is that the ordering of multiple fixed memory windows in that
walk depends on the underlying g_hash_table iterations rather than the
order of creation. In the memory map layout and ACPI table creation we
need both stable and predictable ordering. Resolve this in a similar
fashion to object_class_get_list_sorted() be throwing them in a GSList
and sorting that. Only use this when a sorted list is needed.
Dropped RFC as now I'm happy with this code and would like to get it
upstream! Particularly as it broken even today due to enscripten
related changes that stop us using g_slist_sort(). Easy fix though.
Note that we have an issue for CXL emulation in general and TCG which
is being discussed in:
https://lore.kernel.org/all/20250425183524.00000b28@huawei.com/
(also affects some other platforms)
Until that is resolved, either rebase this back on 10.0 or just
don't let code run out of it (don't use KMEM to expose it as normal
memory, use DAX instead).
Previous cover letter.
Back in 2022, this series stalled on the absence of a solution to device
tree support for PCI Expander Bridges (PXB) and we ended up only having
x86 support upstream. I've been carrying the arm64 support out of tree
since then, with occasional nasty surprises (e.g. UNIMP + DT issue seen
a few weeks ago) and a fair number of fiddly rebases.
gitlab.com/jic23/qemu cxl-<latest date>
A recent discussion with Peter Maydell indicated that there are various
other ACPI only features now, so in general he might be more relaxed
about DT support being necessary. The upcoming vSMMUv3 support would
run into this problem as well.
I presented the background to the PXB issue at Linaro connect 2022. In
short the issue is that PXBs steal MMIO space from the main PCI root
bridge. The challenge is knowing how much to steal.
On ACPI platforms, we can rely on EDK2 to perform an enumeration and
configuration of the PCI topology and QEMU can update the ACPI tables
after EDK2 has done this when it can simply read the space used by the
root ports. On device tree, there is no entity to figure out that
enumeration so we don't know how to size the stolen region.
Three approaches were discussed:
1) Enumerating in QEMU. Horribly complex and the last thing we want is a
3rd enumeration implementation that ends up out of sync with EDK2 and
the kernel (there are frequent issues because of how those existing
implementations differ.
2) Figure out how to enumerate in kernel. I never put a huge amount of work
into this, but it seemed likely to involve a nasty dance with similar
very specific code to that EDK2 is carrying and would very challenging
to upstream (given the lack of clarity on real use cases for PXBs and
DT).
3) Hack it based on the control we have which is bus numbers.
No one liked this but it worked :)
The other little wrinkle would be the need to define full bindings for CXL
on DT + implement a fairly complex kernel stack as equivalent in ACPI
involves a static table, CEDT, new runtime queries via _DSM and a description
of various components. Doable, but so far there is no interest on physical
platforms. Worth noting that for now, the QEMU CXL emulation is all about
testing and developing the OS stack, not about virtualization (performance
is terrible except in some very contrived situations!)
Back to posting as an RFC because there was some discussion of approach to
modelling the devices that may need a bit of redesign.
The discussion kind of died out on the back of DT issue and I doubt anyone
can remember the details.
https://lore.kernel.org/qemu-devel/20220616141950.23374-1-Jonathan.Cameron@huawei.com/
There is only a very simple test in here, because my intent is not to
duplicate what we have on x86, but just to do a smoke test that everything
is hooked up. In general we need much more comprehensive end to end CXL
tests but that requires a reaonsably stable guest software stack. A few
people have expressed interest in working on that, but we aren't there yet.
Note that this series has a very different use case to that in the proposed
SBSA-ref support:
https://lore.kernel.org/qemu-devel/20250117034343.26356-1-wangyuquan1236@phytium.com.cn/
SBSA-ref is a good choice if you want a relatively simple mostly fixed
configuration. That works well with the limited host system
discoverability etc as EDK2 can be build against a known configuration.
My interest with this support in arm/virt is support host software stack
development (we have a wide range of contributors, most of whom are working
on emulation + the kernel support). I care about the weird corners. As such
I need to be able to bring up variable numbers of host bridges, multiple CXL
Fixed Memory Windows with varying characteristics (interleave etc), complex
NUMA topologies with wierd performance characteristics etc. We can do that
on x86 upstream today, or my gitlab tree. Note that we need arm support
for some arch specific features in the near future (cache flushing).
Doing kernel development with this need for flexibility on SBSA-ref is not
currently practical. SBSA-ref CXL support is an excellent thing, just
not much use to me for this work.
Jonathan Cameron (5):
hw/cxl-host: Add an index field to CXLFixedMemoryWindow
hw/cxl: Make the CXL fixed memory windows devices.
hw/cxl-host: Allow split of establishing memory address and mmio
setup.
hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances
pxb-cxl
qtest/cxl: Add aarch64 virt test for CXL
include/hw/arm/virt.h | 4 +
include/hw/cxl/cxl.h | 4 +
include/hw/cxl/cxl_host.h | 6 +-
hw/acpi/cxl.c | 83 +++++++++------
hw/arm/virt-acpi-build.c | 34 ++++++
hw/arm/virt.c | 29 +++++
hw/cxl/cxl-host-stubs.c | 8 +-
hw/cxl/cxl-host.c | 218 ++++++++++++++++++++++++++++++++------
hw/i386/pc.c | 51 ++++-----
tests/qtest/cxl-test.c | 59 ++++++++---
tests/qtest/meson.build | 1 +
11 files changed, 389 insertions(+), 108 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow
2025-05-13 11:14 [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Jonathan Cameron
@ 2025-05-13 11:14 ` Jonathan Cameron
2025-05-16 5:44 ` Zhijian Li (Fujitsu)
` (3 more replies)
2025-05-13 11:14 ` [PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices Jonathan Cameron
` (4 subsequent siblings)
5 siblings, 4 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-13 11:14 UTC (permalink / raw)
To: qemu-devel, Fan Ni, Peter Maydell, mst
Cc: linux-cxl, linuxarm, qemu-arm, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
To enable these to be found in a fixed order, that order needs
to be known. This will later be used to sort a list of these
structures that address map and ACPI table entries are predictable.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
include/hw/cxl/cxl.h | 1 +
hw/cxl/cxl-host.c | 9 ++++++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 75e47b6864..b2bcce7ed6 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -27,6 +27,7 @@
typedef struct PXBCXLDev PXBCXLDev;
typedef struct CXLFixedWindow {
+ int index;
uint64_t size;
char **targets;
PXBCXLDev *target_hbs[16];
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index e010163174..b7aa429ddf 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -24,13 +24,15 @@
static void cxl_fixed_memory_window_config(CXLState *cxl_state,
CXLFixedMemoryWindowOptions *object,
- Error **errp)
+ int index, Error **errp)
{
ERRP_GUARD();
g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
strList *target;
int i;
+ fw->index = index;
+
for (target = object->targets; target; target = target->next) {
fw->num_targets++;
}
@@ -325,14 +327,15 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
CXLState *state = opaque;
CXLFixedMemoryWindowOptionsList *cfmw_list = NULL;
CXLFixedMemoryWindowOptionsList *it;
+ int index;
visit_type_CXLFixedMemoryWindowOptionsList(v, name, &cfmw_list, errp);
if (!cfmw_list) {
return;
}
- for (it = cfmw_list; it; it = it->next) {
- cxl_fixed_memory_window_config(state, it->value, errp);
+ for (it = cfmw_list, index = 0; it; it = it->next, index++) {
+ cxl_fixed_memory_window_config(state, it->value, index, errp);
}
state->cfmw_list = cfmw_list;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices.
2025-05-13 11:14 [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow Jonathan Cameron
@ 2025-05-13 11:14 ` Jonathan Cameron
2025-05-16 5:44 ` Zhijian Li (Fujitsu)
2025-05-13 11:14 ` [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup Jonathan Cameron
` (3 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-13 11:14 UTC (permalink / raw)
To: qemu-devel, Fan Ni, Peter Maydell, mst
Cc: linux-cxl, linuxarm, qemu-arm, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
Previously these somewhat device like structures were tracked using a list
in the CXLState in each machine. This is proving restrictive in a few
cases where we need to iterate through these without being aware of the
machine type. Just make them sysbus devices.
Restrict them to not user created as they need to be visible to early
stages of machine init given effects on the memory map.
This change both simplifies state tracking and enables features needed
for performance optimization and hotness tracking by making it possible
to retrieve the fixed memory window on actions elsewhere in the topology.
In some cases the ordering of the Fixed Memory Windows matters.
For those utility functions provide a GSList sorted by the window index.
This ensures that we get consistency across:
- ordering in the command line
- ordering of the host PA ranges
- ordering of ACPI CEDT structures describing the CFMWS.
Other aspects don't have this constraint. For those direct iteration
of the underlying hash structures is fine.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
I think Peter Maydell suggested this a long time back when
the original CXL support series was under review but not 100% sure.
---
include/hw/cxl/cxl.h | 3 +
include/hw/cxl/cxl_host.h | 4 +-
hw/acpi/cxl.c | 83 +++++++++++--------
hw/cxl/cxl-host-stubs.c | 6 +-
hw/cxl/cxl-host.c | 169 +++++++++++++++++++++++++++++++-------
hw/i386/pc.c | 51 ++++++------
6 files changed, 223 insertions(+), 93 deletions(-)
diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index b2bcce7ed6..a610795c87 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -27,6 +27,7 @@
typedef struct PXBCXLDev PXBCXLDev;
typedef struct CXLFixedWindow {
+ SysBusDevice parent_obj;
int index;
uint64_t size;
char **targets;
@@ -38,6 +39,8 @@ typedef struct CXLFixedWindow {
MemoryRegion mr;
hwaddr base;
} CXLFixedWindow;
+#define TYPE_CXL_FMW "cxl-fmw"
+OBJECT_DECLARE_SIMPLE_TYPE(CXLFixedWindow, CXL_FMW)
typedef struct CXLState {
bool is_enabled;
diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
index c9bc9c7c50..6dce2cde07 100644
--- a/include/hw/cxl/cxl_host.h
+++ b/include/hw/cxl/cxl_host.h
@@ -14,8 +14,10 @@
#define CXL_HOST_H
void cxl_machine_init(Object *obj, CXLState *state);
-void cxl_fmws_link_targets(CXLState *stat, Error **errp);
+void cxl_fmws_link_targets(Error **errp);
void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp);
+hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr);
+GSList *cxl_fmws_get_all_sorted(void);
extern const MemoryRegionOps cfmws_ops;
diff --git a/hw/acpi/cxl.c b/hw/acpi/cxl.c
index 9cd7905ea2..20806e5dd4 100644
--- a/hw/acpi/cxl.c
+++ b/hw/acpi/cxl.c
@@ -22,6 +22,7 @@
#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_host.h"
#include "hw/cxl/cxl.h"
+#include "hw/cxl/cxl_host.h"
#include "hw/mem/memory-device.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/aml-build.h"
@@ -135,56 +136,62 @@ static void cedt_build_chbs(GArray *table_data, PXBCXLDev *cxl)
* Interleave ways encoding in CXL 2.0 ECN: 3, 6, 12 and 16-way memory
* interleaving.
*/
-static void cedt_build_cfmws(GArray *table_data, CXLState *cxls)
+static int cedt_build_cfmws(Object *obj, void *opaque)
{
- GList *it;
+ struct CXLFixedWindow *fw;
+ Aml *cedt = opaque;
+ GArray *table_data = cedt->buf;
+ int i;
- for (it = cxls->fixed_windows; it; it = it->next) {
- CXLFixedWindow *fw = it->data;
- int i;
+ if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
+ return 0;
+ }
+ fw = CXL_FMW(obj);
- /* Type */
- build_append_int_noprefix(table_data, 1, 1);
+ /* Type */
+ build_append_int_noprefix(table_data, 1, 1);
- /* Reserved */
- build_append_int_noprefix(table_data, 0, 1);
+ /* Reserved */
+ build_append_int_noprefix(table_data, 0, 1);
- /* Record Length */
- build_append_int_noprefix(table_data, 36 + 4 * fw->num_targets, 2);
+ /* Record Length */
+ build_append_int_noprefix(table_data, 36 + 4 * fw->num_targets, 2);
- /* Reserved */
- build_append_int_noprefix(table_data, 0, 4);
+ /* Reserved */
+ build_append_int_noprefix(table_data, 0, 4);
- /* Base HPA */
- build_append_int_noprefix(table_data, fw->mr.addr, 8);
+ /* Base HPA */
+ build_append_int_noprefix(table_data, fw->mr.addr, 8);
- /* Window Size */
- build_append_int_noprefix(table_data, fw->size, 8);
+ /* Window Size */
+ build_append_int_noprefix(table_data, fw->size, 8);
- /* Host Bridge Interleave Ways */
- build_append_int_noprefix(table_data, fw->enc_int_ways, 1);
+ /* Host Bridge Interleave Ways */
+ build_append_int_noprefix(table_data, fw->enc_int_ways, 1);
- /* Host Bridge Interleave Arithmetic */
- build_append_int_noprefix(table_data, 0, 1);
+ /* Host Bridge Interleave Arithmetic */
+ build_append_int_noprefix(table_data, 0, 1);
- /* Reserved */
- build_append_int_noprefix(table_data, 0, 2);
+ /* Reserved */
+ build_append_int_noprefix(table_data, 0, 2);
- /* Host Bridge Interleave Granularity */
- build_append_int_noprefix(table_data, fw->enc_int_gran, 4);
+ /* Host Bridge Interleave Granularity */
+ build_append_int_noprefix(table_data, fw->enc_int_gran, 4);
- /* Window Restrictions */
- build_append_int_noprefix(table_data, 0x0f, 2); /* No restrictions */
+ /* Window Restrictions */
+ build_append_int_noprefix(table_data, 0x0f, 2);
- /* QTG ID */
- build_append_int_noprefix(table_data, 0, 2);
+ /* QTG ID */
+ build_append_int_noprefix(table_data, 0, 2);
- /* Host Bridge List (list of UIDs - currently bus_nr) */
- for (i = 0; i < fw->num_targets; i++) {
- g_assert(fw->target_hbs[i]);
- build_append_int_noprefix(table_data, PXB_DEV(fw->target_hbs[i])->bus_nr, 4);
- }
+ /* Host Bridge List (list of UIDs - currently bus_nr) */
+ for (i = 0; i < fw->num_targets; i++) {
+ g_assert(fw->target_hbs[i]);
+ build_append_int_noprefix(table_data,
+ PXB_DEV(fw->target_hbs[i])->bus_nr, 4);
}
+
+ return 0;
}
static int cxl_foreach_pxb_hb(Object *obj, void *opaque)
@@ -202,6 +209,7 @@ void cxl_build_cedt(GArray *table_offsets, GArray *table_data,
BIOSLinker *linker, const char *oem_id,
const char *oem_table_id, CXLState *cxl_state)
{
+ GSList *cfmws_list, *iter;
Aml *cedt;
AcpiTable table = { .sig = "CEDT", .rev = 1, .oem_id = oem_id,
.oem_table_id = oem_table_id };
@@ -213,7 +221,12 @@ void cxl_build_cedt(GArray *table_offsets, GArray *table_data,
/* reserve space for CEDT header */
object_child_foreach_recursive(object_get_root(), cxl_foreach_pxb_hb, cedt);
- cedt_build_cfmws(cedt->buf, cxl_state);
+
+ cfmws_list = cxl_fmws_get_all_sorted();
+ for (iter = cfmws_list; iter; iter = iter->next) {
+ cedt_build_cfmws(iter->data, cedt);
+ }
+ g_slist_free(cfmws_list);
/* copy AML table into ACPI tables blob and patch header there */
g_array_append_vals(table_data, cedt->buf->data, cedt->buf->len);
diff --git a/hw/cxl/cxl-host-stubs.c b/hw/cxl/cxl-host-stubs.c
index cae4afcdde..13eb6bf6a4 100644
--- a/hw/cxl/cxl-host-stubs.c
+++ b/hw/cxl/cxl-host-stubs.c
@@ -8,8 +8,12 @@
#include "hw/cxl/cxl.h"
#include "hw/cxl/cxl_host.h"
-void cxl_fmws_link_targets(CXLState *stat, Error **errp) {};
+void cxl_fmws_link_targets(Error **errp) {};
void cxl_machine_init(Object *obj, CXLState *state) {};
void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp) {};
+hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
+{
+ return base;
+};
const MemoryRegionOps cfmws_ops;
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index b7aa429ddf..438f2920e1 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -22,12 +22,12 @@
#include "hw/pci/pcie_port.h"
#include "hw/pci-bridge/pci_expander_bridge.h"
-static void cxl_fixed_memory_window_config(CXLState *cxl_state,
- CXLFixedMemoryWindowOptions *object,
+static void cxl_fixed_memory_window_config(CXLFixedMemoryWindowOptions *object,
int index, Error **errp)
{
ERRP_GUARD();
- g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
+ DeviceState *dev = qdev_new(TYPE_CXL_FMW);
+ CXLFixedWindow *fw = CXL_FMW(dev);
strList *target;
int i;
@@ -67,35 +67,41 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state,
fw->targets[i] = g_strdup(target->value);
}
- cxl_state->fixed_windows = g_list_append(cxl_state->fixed_windows,
- g_steal_pointer(&fw));
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp);
+
+ return;
}
-void cxl_fmws_link_targets(CXLState *cxl_state, Error **errp)
+static int cxl_fmws_link(Object *obj, void *opaque)
{
- if (cxl_state && cxl_state->fixed_windows) {
- GList *it;
-
- for (it = cxl_state->fixed_windows; it; it = it->next) {
- CXLFixedWindow *fw = it->data;
- int i;
-
- for (i = 0; i < fw->num_targets; i++) {
- Object *o;
- bool ambig;
-
- o = object_resolve_path_type(fw->targets[i],
- TYPE_PXB_CXL_DEV,
- &ambig);
- if (!o) {
- error_setg(errp, "Could not resolve CXLFM target %s",
- fw->targets[i]);
- return;
- }
- fw->target_hbs[i] = PXB_CXL_DEV(o);
- }
+ struct CXLFixedWindow *fw;
+ int i;
+
+ if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
+ return 0;
+ }
+ fw = CXL_FMW(obj);
+
+ for (i = 0; i < fw->num_targets; i++) {
+ Object *o;
+ bool ambig;
+
+ o = object_resolve_path_type(fw->targets[i], TYPE_PXB_CXL_DEV,
+ &ambig);
+ if (!o) {
+ error_setg(&error_fatal, "Could not resolve CXLFM target %s",
+ fw->targets[i]);
+ return 1;
}
+ fw->target_hbs[i] = PXB_CXL_DEV(o);
}
+ return 0;
+}
+
+void cxl_fmws_link_targets(Error **errp)
+{
+ /* Order doesn't matter for this, so no need to build list */
+ object_child_foreach_recursive(object_get_root(), cxl_fmws_link, NULL);
}
static bool cxl_hdm_find_target(uint32_t *cache_mem, hwaddr addr,
@@ -335,7 +341,7 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
}
for (it = cfmw_list, index = 0; it; it = it->next, index++) {
- cxl_fixed_memory_window_config(state, it->value, index, errp);
+ cxl_fixed_memory_window_config(it->value, index, errp);
}
state->cfmw_list = cfmw_list;
}
@@ -373,3 +379,110 @@ void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp)
}
}
}
+
+struct cfmw_update_state {
+ hwaddr base;
+ hwaddr maxaddr;
+};
+
+static void cxl_fmws_update(Object *obj, void *opaque)
+{
+ struct CXLFixedWindow *fw;
+ struct cfmw_update_state *s = opaque;
+
+ if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
+ return;
+ }
+
+ fw = CXL_FMW(obj);
+ if (s->base + fw->size <= s->maxaddr) {
+ fw->base = s->base;
+ sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
+ s->base += fw->size;
+ }
+
+ return;
+}
+
+static int cxl_fmws_find(Object *obj, void *opaque)
+{
+ GSList **list = opaque;
+
+ if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
+ return 0;
+ }
+ *list = g_slist_prepend(*list, obj);
+
+ return 0;
+}
+
+static GSList *cxl_fmws_get_all(void)
+{
+ GSList *list = NULL;
+
+ object_child_foreach_recursive(object_get_root(), cxl_fmws_find, &list);
+
+ return list;
+}
+
+static gint cfmws_cmp(gconstpointer a, gconstpointer b, gpointer d)
+{
+ const struct CXLFixedWindow *ap = a;
+ const struct CXLFixedWindow *bp = b;
+
+ return ap->index > bp->index;
+}
+
+GSList *cxl_fmws_get_all_sorted(void)
+{
+ return g_slist_sort_with_data(cxl_fmws_get_all(), cfmws_cmp, NULL);
+}
+
+hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
+{
+ GSList *cfmws_list, *iter;
+
+ struct cfmw_update_state cfmwss = {
+ .base = base,
+ .maxaddr = max_addr,
+ };
+ cfmws_list = cxl_fmws_get_all_sorted();
+ for (iter = cfmws_list; iter; iter = iter->next) {
+ cxl_fmws_update(iter->data, &cfmwss);
+ }
+ g_slist_free(cfmws_list);
+
+ return cfmwss.base;
+}
+
+static void cxl_fmw_realize(DeviceState *dev, Error **errp)
+{
+ CXLFixedWindow *fw = CXL_FMW(dev);
+
+ memory_region_init_io(&fw->mr, OBJECT(dev), &cfmws_ops, fw,
+ "cxl-fixed-memory-region", fw->size);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &fw->mr);
+}
+
+static void cxl_fmw_class_init(ObjectClass *klass, const void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->desc = "CXL Fixed Memory Window";
+ dc->realize = cxl_fmw_realize;
+ /* Reason - created by machines as tightly coupled to machine memory map */
+ dc->user_creatable = false;
+}
+
+static const TypeInfo cxl_fmw_info = {
+ .name = TYPE_CXL_FMW,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(CXLFixedWindow),
+ .class_init = cxl_fmw_class_init,
+};
+
+static void cxl_host_register_types(void)
+{
+ type_register_static(&cxl_fmw_info);
+}
+type_init(cxl_host_register_types)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 70656157ca..9978398876 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -630,7 +630,7 @@ void pc_machine_done(Notifier *notifier, void *data)
&error_fatal);
if (pcms->cxl_devices_state.is_enabled) {
- cxl_fmws_link_targets(&pcms->cxl_devices_state, &error_fatal);
+ cxl_fmws_link_targets(&error_fatal);
}
/* set the number of CPUs */
@@ -739,20 +739,28 @@ static uint64_t pc_get_cxl_range_start(PCMachineState *pcms)
return cxl_base;
}
-static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
+static int cxl_get_fmw_end(Object *obj, void *opaque)
{
- uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
+ struct CXLFixedWindow *fw;
+ uint64_t *start = opaque;
- if (pcms->cxl_devices_state.fixed_windows) {
- GList *it;
-
- start = ROUND_UP(start, 256 * MiB);
- for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
- CXLFixedWindow *fw = it->data;
- start += fw->size;
- }
+ if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
+ return 0;
}
+ fw = CXL_FMW(obj);
+
+ *start += fw->size;
+ return 0;
+}
+
+static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
+{
+ uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
+
+ /* Ordering doesn't matter so no need to build a sorted list */
+ object_child_foreach_recursive(object_get_root(), cxl_get_fmw_end,
+ &start);
return start;
}
@@ -954,23 +962,10 @@ void pc_memory_init(PCMachineState *pcms,
cxl_base = pc_get_cxl_range_start(pcms);
memory_region_init(mr, OBJECT(machine), "cxl_host_reg", cxl_size);
memory_region_add_subregion(system_memory, cxl_base, mr);
- cxl_resv_end = cxl_base + cxl_size;
- if (pcms->cxl_devices_state.fixed_windows) {
- hwaddr cxl_fmw_base;
- GList *it;
-
- cxl_fmw_base = ROUND_UP(cxl_base + cxl_size, 256 * MiB);
- for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
- CXLFixedWindow *fw = it->data;
-
- fw->base = cxl_fmw_base;
- memory_region_init_io(&fw->mr, OBJECT(machine), &cfmws_ops, fw,
- "cxl-fixed-memory-region", fw->size);
- memory_region_add_subregion(system_memory, fw->base, &fw->mr);
- cxl_fmw_base += fw->size;
- cxl_resv_end = cxl_fmw_base;
- }
- }
+ cxl_base = ROUND_UP(cxl_base + cxl_size, 256 * MiB);
+
+ cxl_resv_end = cxl_fmws_set_memmap_and_update_mmio(cxl_base,
+ maxphysaddr);
}
/* Initialize PC system firmware */
--
2.43.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup.
2025-05-13 11:14 [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices Jonathan Cameron
@ 2025-05-13 11:14 ` Jonathan Cameron
2025-05-16 5:50 ` Zhijian Li (Fujitsu)
2025-05-13 11:14 ` [PATCH v13 4/5] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl Jonathan Cameron
` (2 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-13 11:14 UTC (permalink / raw)
To: qemu-devel, Fan Ni, Peter Maydell, mst
Cc: linux-cxl, linuxarm, qemu-arm, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
On arm/virt the memory map is set up before any devices are brought
up. To enable this provide split functions to establish the fw->base
and later to actually map it.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
include/hw/cxl/cxl_host.h | 2 ++
hw/cxl/cxl-host-stubs.c | 2 ++
hw/cxl/cxl-host.c | 44 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
index 6dce2cde07..aee9d573d6 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(Error **errp);
void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp);
+hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr);
+void cxl_fmws_update_mmio(void);
hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr);
GSList *cxl_fmws_get_all_sorted(void);
diff --git a/hw/cxl/cxl-host-stubs.c b/hw/cxl/cxl-host-stubs.c
index 13eb6bf6a4..d9e38618d6 100644
--- a/hw/cxl/cxl-host-stubs.c
+++ b/hw/cxl/cxl-host-stubs.c
@@ -11,6 +11,8 @@
void cxl_fmws_link_targets(Error **errp) {};
void cxl_machine_init(Object *obj, CXLState *state) {};
void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp) {};
+hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr) { return base; };
+void cxl_fmws_update_mmio(void) {};
hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
{
return base;
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 438f2920e1..3b9184e7e7 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -383,6 +383,7 @@ void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp)
struct cfmw_update_state {
hwaddr base;
hwaddr maxaddr;
+ bool update_mmio;
};
static void cxl_fmws_update(Object *obj, void *opaque)
@@ -397,7 +398,9 @@ static void cxl_fmws_update(Object *obj, void *opaque)
fw = CXL_FMW(obj);
if (s->base + fw->size <= s->maxaddr) {
fw->base = s->base;
- sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
+ if (s->update_mmio) {
+ sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
+ }
s->base += fw->size;
}
@@ -438,6 +441,24 @@ GSList *cxl_fmws_get_all_sorted(void)
return g_slist_sort_with_data(cxl_fmws_get_all(), cfmws_cmp, NULL);
}
+hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr)
+{
+ GSList *cfmws_list, *iter;
+
+ struct cfmw_update_state cfmwss = {
+ .base = base,
+ .maxaddr = max_addr,
+ .update_mmio = false,
+ };
+ cfmws_list = cxl_fmws_get_all_sorted();
+ for (iter = cfmws_list; iter; iter = iter->next) {
+ cxl_fmws_update(iter->data, &cfmwss);
+ }
+ g_slist_free(cfmws_list);
+
+ return cfmwss.base;
+}
+
hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
{
GSList *cfmws_list, *iter;
@@ -445,6 +466,7 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
struct cfmw_update_state cfmwss = {
.base = base,
.maxaddr = max_addr,
+ .update_mmio = true,
};
cfmws_list = cxl_fmws_get_all_sorted();
for (iter = cfmws_list; iter; iter = iter->next) {
@@ -455,6 +477,26 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
return cfmwss.base;
}
+static int cxl_fmws_mmio_map(Object *obj, void *opaque)
+{
+ struct CXLFixedWindow *fw;
+
+ if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
+ return 0;
+ }
+ fw = CXL_FMW(obj);
+ sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
+
+ return 0;
+}
+
+void cxl_fmws_update_mmio(void)
+{
+ /* Ordering is not required for this */
+ object_child_foreach_recursive(object_get_root(), cxl_fmws_mmio_map,
+ NULL);
+}
+
static void cxl_fmw_realize(DeviceState *dev, Error **errp)
{
CXLFixedWindow *fw = CXL_FMW(dev);
--
2.43.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v13 4/5] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl
2025-05-13 11:14 [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Jonathan Cameron
` (2 preceding siblings ...)
2025-05-13 11:14 ` [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup Jonathan Cameron
@ 2025-05-13 11:14 ` Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL Jonathan Cameron
2025-05-16 2:30 ` [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Itaru Kitayama
5 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-13 11:14 UTC (permalink / raw)
To: qemu-devel, Fan Ni, Peter Maydell, mst
Cc: linux-cxl, linuxarm, qemu-arm, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
Code based on i386/pc enablement.
The memory layout places space for 16 host bridge register regions after
the GIC_REDIST2 in the extended memmap.
The CFMWs are placed above the extended memmap.
Only create the CEDT table if cxl=on set for the machine.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
include/hw/arm/virt.h | 4 ++++
hw/arm/virt-acpi-build.c | 34 ++++++++++++++++++++++++++++++++++
hw/arm/virt.c | 29 +++++++++++++++++++++++++++++
3 files changed, 67 insertions(+)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 9a1b0f53d2..4375819ea0 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -36,6 +36,7 @@
#include "hw/arm/boot.h"
#include "hw/arm/bsa.h"
#include "hw/block/flash.h"
+#include "hw/cxl/cxl.h"
#include "system/kvm.h"
#include "hw/intc/arm_gicv3_common.h"
#include "qom/object.h"
@@ -85,6 +86,7 @@ enum {
/* indices of IO regions located after the RAM */
enum {
VIRT_HIGH_GIC_REDIST2 = VIRT_LOWMEMMAP_LAST,
+ VIRT_CXL_HOST,
VIRT_HIGH_PCIE_ECAM,
VIRT_HIGH_PCIE_MMIO,
};
@@ -140,6 +142,7 @@ struct VirtMachineState {
bool secure;
bool highmem;
bool highmem_compact;
+ bool highmem_cxl;
bool highmem_ecam;
bool highmem_mmio;
bool highmem_redists;
@@ -174,6 +177,7 @@ struct VirtMachineState {
char *oem_id;
char *oem_table_id;
bool ns_el2_virt_timer_irq;
+ CXLState cxl_devices_state;
};
#define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7e8e0f0298..589e221b89 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -39,10 +39,12 @@
#include "hw/acpi/aml-build.h"
#include "hw/acpi/utils.h"
#include "hw/acpi/pci.h"
+#include "hw/acpi/cxl.h"
#include "hw/acpi/memory_hotplug.h"
#include "hw/acpi/generic_event_device.h"
#include "hw/acpi/tpm.h"
#include "hw/acpi/hmat.h"
+#include "hw/cxl/cxl.h"
#include "hw/pci/pcie_host.h"
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
@@ -119,10 +121,29 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
aml_append(scope, dev);
}
+static void build_acpi0017(Aml *table)
+{
+ Aml *dev, *scope, *method;
+
+ scope = aml_scope("_SB");
+ dev = aml_device("CXLM");
+ aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0017")));
+
+ method = aml_method("_STA", 0, AML_NOTSERIALIZED);
+ aml_append(method, aml_return(aml_int(0x0B)));
+ aml_append(dev, method);
+ build_cxl_dsm_method(dev);
+
+ aml_append(scope, dev);
+ aml_append(table, scope);
+}
+
static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
uint32_t irq, VirtMachineState *vms)
{
int ecam_id = VIRT_ECAM_ID(vms->highmem_ecam);
+ bool cxl_present = false;
+ PCIBus *bus = vms->bus;
struct GPEXConfig cfg = {
.mmio32 = memmap[VIRT_PCIE_MMIO],
.pio = memmap[VIRT_PCIE_PIO],
@@ -136,6 +157,14 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
}
acpi_dsdt_add_gpex(scope, &cfg);
+ QLIST_FOREACH(bus, &vms->bus->child, sibling) {
+ if (pci_bus_is_cxl(bus)) {
+ cxl_present = true;
+ }
+ }
+ if (cxl_present) {
+ build_acpi0017(scope);
+ }
}
static void acpi_dsdt_add_gpio(Aml *scope, const MemMapEntry *gpio_memmap,
@@ -963,6 +992,11 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
}
}
+ if (vms->cxl_devices_state.is_enabled) {
+ cxl_build_cedt(table_offsets, tables_blob, tables->linker,
+ vms->oem_id, vms->oem_table_id, &vms->cxl_devices_state);
+ }
+
if (ms->nvdimms_state->is_enabled) {
nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
ms->nvdimms_state, ms->ram_slots, vms->oem_id,
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9a6cd085a3..e06d293edc 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -57,6 +57,7 @@
#include "qemu/error-report.h"
#include "qemu/module.h"
#include "hw/pci-host/gpex.h"
+#include "hw/pci-bridge/pci_expander_bridge.h"
#include "hw/virtio/virtio-pci.h"
#include "hw/core/sysbus-fdt.h"
#include "hw/platform-bus.h"
@@ -86,6 +87,8 @@
#include "hw/virtio/virtio-md-pci.h"
#include "hw/virtio/virtio-iommu.h"
#include "hw/char/pl011.h"
+#include "hw/cxl/cxl.h"
+#include "hw/cxl/cxl_host.h"
#include "qemu/guest-random.h"
static GlobalProperty arm_virt_compat[] = {
@@ -220,6 +223,7 @@ static const MemMapEntry base_memmap[] = {
static MemMapEntry extended_memmap[] = {
/* Additional 64 MB redist region (can contain up to 512 redistributors) */
[VIRT_HIGH_GIC_REDIST2] = { 0x0, 64 * MiB },
+ [VIRT_CXL_HOST] = { 0x0, 64 * KiB * 16 }, /* 16 UID */
[VIRT_HIGH_PCIE_ECAM] = { 0x0, 256 * MiB },
/* Second PCIe window */
[VIRT_HIGH_PCIE_MMIO] = { 0x0, DEFAULT_HIGH_PCIE_MMIO_SIZE },
@@ -1621,6 +1625,17 @@ static void create_pcie(VirtMachineState *vms)
}
}
+static void create_cxl_host_reg_region(VirtMachineState *vms)
+{
+ MemoryRegion *sysmem = get_system_memory();
+ MemoryRegion *mr = &vms->cxl_devices_state.host_mr;
+
+ memory_region_init(mr, OBJECT(vms), "cxl_host_reg",
+ vms->memmap[VIRT_CXL_HOST].size);
+ memory_region_add_subregion(sysmem, vms->memmap[VIRT_CXL_HOST].base, mr);
+ vms->highmem_cxl = true;
+}
+
static void create_platform_bus(VirtMachineState *vms)
{
DeviceState *dev;
@@ -1737,6 +1752,12 @@ void virt_machine_done(Notifier *notifier, void *data)
struct arm_boot_info *info = &vms->bootinfo;
AddressSpace *as = arm_boot_address_space(cpu, info);
+ cxl_hook_up_pxb_registers(vms->bus, &vms->cxl_devices_state,
+ &error_fatal);
+
+ if (vms->cxl_devices_state.is_enabled) {
+ cxl_fmws_link_targets(&error_fatal);
+ }
/*
* If the user provided a dtb, we assume the dynamic sysbus nodes
* already are integrated there. This corresponds to a use case where
@@ -1783,6 +1804,7 @@ static inline bool *virt_get_high_memmap_enabled(VirtMachineState *vms,
{
bool *enabled_array[] = {
&vms->highmem_redists,
+ &vms->highmem_cxl,
&vms->highmem_ecam,
&vms->highmem_mmio,
};
@@ -1890,6 +1912,9 @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
if (device_memory_size > 0) {
machine_memory_devices_init(ms, device_memory_base, device_memory_size);
}
+
+ cxl_fmws_set_memmap(ROUND_UP(vms->highest_gpa + 1, 256 * MiB),
+ BIT_ULL(pa_bits));
}
static VirtGICType finalize_gic_version_do(const char *accel_name,
@@ -2340,6 +2365,8 @@ static void machvirt_init(MachineState *machine)
memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base,
machine->ram);
+ cxl_fmws_update_mmio();
+
virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
create_gic(vms, sysmem);
@@ -2395,6 +2422,7 @@ static void machvirt_init(MachineState *machine)
create_rtc(vms);
create_pcie(vms);
+ create_cxl_host_reg_region(vms);
if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) {
vms->acpi_dev = create_acpi_ged(vms);
@@ -3365,6 +3393,7 @@ static void virt_instance_init(Object *obj)
vms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
vms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
+ cxl_machine_init(obj, &vms->cxl_devices_state);
}
static const TypeInfo virt_machine_info = {
--
2.43.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL
2025-05-13 11:14 [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Jonathan Cameron
` (3 preceding siblings ...)
2025-05-13 11:14 ` [PATCH v13 4/5] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl Jonathan Cameron
@ 2025-05-13 11:14 ` Jonathan Cameron
2025-05-15 9:04 ` Itaru Kitayama
2025-05-16 2:30 ` [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Itaru Kitayama
5 siblings, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-13 11:14 UTC (permalink / raw)
To: qemu-devel, Fan Ni, Peter Maydell, mst
Cc: linux-cxl, linuxarm, qemu-arm, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
Add a single complex case for aarch64 virt machine.
Given existing much more comprehensive tests for x86 cover the
common functionality, a single test should be enough to verify
that the aarch64 part continue to work.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
tests/qtest/cxl-test.c | 59 ++++++++++++++++++++++++++++++++---------
tests/qtest/meson.build | 1 +
2 files changed, 47 insertions(+), 13 deletions(-)
diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
index a600331843..c7189d6222 100644
--- a/tests/qtest/cxl-test.c
+++ b/tests/qtest/cxl-test.c
@@ -19,6 +19,12 @@
"-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
"-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
+#define QEMU_VIRT_2PXB_CMD \
+ "-machine virt,cxl=on -cpu max " \
+ "-device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52 " \
+ "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
+ "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
+
#define QEMU_RP \
"-device cxl-rp,id=rp0,bus=cxl.0,chassis=0,slot=0 "
@@ -197,25 +203,52 @@ static void cxl_2pxb_4rp_4t3d(void)
qtest_end();
rmdir(tmpfs);
}
+
+static void cxl_virt_2pxb_4rp_4t3d(void)
+{
+ g_autoptr(GString) cmdline = g_string_new(NULL);
+ char template[] = "/tmp/cxl-test-XXXXXX";
+ const char *tmpfs;
+
+ tmpfs = mkdtemp(template);
+
+ g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
+ tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
+ tmpfs, tmpfs);
+
+ qtest_start(cmdline->str);
+ qtest_end();
+ rmdir(tmpfs);
+}
#endif /* CONFIG_POSIX */
int main(int argc, char **argv)
{
- g_test_init(&argc, &argv, NULL);
+ const char *arch = qtest_get_arch();
- qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
- qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
- qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
- qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
- qtest_add_func("/pci/cxl/rp", cxl_root_port);
- qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
+ g_test_init(&argc, &argv, NULL);
+ if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+ qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
+ qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
+ qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
+ qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
+ qtest_add_func("/pci/cxl/rp", cxl_root_port);
+ qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
#ifdef CONFIG_POSIX
- qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
- qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
- qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
- qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
- qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
- qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4", cxl_2pxb_4rp_4t3d);
+ qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
+ qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
+ qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
+ qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
+ qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
+ qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4",
+ cxl_2pxb_4rp_4t3d);
#endif
+ } else if (strcmp(arch, "aarch64") == 0) {
+#ifdef CONFIG_POSIX
+ qtest_add_func("/pci/cxl/virt/pxb_x2_root_port_x4_type3_x4",
+ cxl_virt_2pxb_4rp_4t3d);
+#endif
+ }
+
return g_test_run();
}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 7daf619845..361000267a 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -258,6 +258,7 @@ qtests_aarch64 = \
(config_all_accel.has_key('CONFIG_TCG') and \
config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
(config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
+ qtests_cxl + \
['arm-cpu-features',
'numa-test',
'boot-serial-test',
--
2.43.0
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL
2025-05-13 11:14 ` [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL Jonathan Cameron
@ 2025-05-15 9:04 ` Itaru Kitayama
2025-05-19 12:54 ` Jonathan Cameron
0 siblings, 1 reply; 24+ messages in thread
From: Itaru Kitayama @ 2025-05-15 9:04 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
> On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>
> Add a single complex case for aarch64 virt machine.
> Given existing much more comprehensive tests for x86 cover the
> common functionality, a single test should be enough to verify
> that the aarch64 part continue to work.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> tests/qtest/cxl-test.c | 59 ++++++++++++++++++++++++++++++++---------
> tests/qtest/meson.build | 1 +
> 2 files changed, 47 insertions(+), 13 deletions(-)
>
> diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
> index a600331843..c7189d6222 100644
> --- a/tests/qtest/cxl-test.c
> +++ b/tests/qtest/cxl-test.c
> @@ -19,6 +19,12 @@
> "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
> "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
>
> +#define QEMU_VIRT_2PXB_CMD \
> + "-machine virt,cxl=on -cpu max " \
> + "-device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52 " \
> + "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
> + "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
> +
> #define QEMU_RP \
> "-device cxl-rp,id=rp0,bus=cxl.0,chassis=0,slot=0 "
>
> @@ -197,25 +203,52 @@ static void cxl_2pxb_4rp_4t3d(void)
> qtest_end();
> rmdir(tmpfs);
> }
> +
> +static void cxl_virt_2pxb_4rp_4t3d(void)
> +{
> + g_autoptr(GString) cmdline = g_string_new(NULL);
> + char template[] = "/tmp/cxl-test-XXXXXX";
> + const char *tmpfs;
> +
> + tmpfs = mkdtemp(template);
> +
> + g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
> + tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
> + tmpfs, tmpfs);
> +
> + qtest_start(cmdline->str);
> + qtest_end();
> + rmdir(tmpfs);
> +}
> #endif /* CONFIG_POSIX */
>
> int main(int argc, char **argv)
> {
> - g_test_init(&argc, &argv, NULL);
> + const char *arch = qtest_get_arch();
>
> - qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
> - qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
> - qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
> - qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
> - qtest_add_func("/pci/cxl/rp", cxl_root_port);
> - qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
> + g_test_init(&argc, &argv, NULL);
> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> + qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
> + qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
> + qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
> + qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
> + qtest_add_func("/pci/cxl/rp", cxl_root_port);
> + qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
> #ifdef CONFIG_POSIX
> - qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
> - qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
> - qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
> - qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
> - qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
> - qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4", cxl_2pxb_4rp_4t3d);
> + qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
> + qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
> + qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
> + qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
> + qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
> + qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4",
> + cxl_2pxb_4rp_4t3d);
> #endif
> + } else if (strcmp(arch, "aarch64") == 0) {
> +#ifdef CONFIG_POSIX
> + qtest_add_func("/pci/cxl/virt/pxb_x2_root_port_x4_type3_x4",
> + cxl_virt_2pxb_4rp_4t3d);
> +#endif
> + }
> +
> return g_test_run();
> }
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 7daf619845..361000267a 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -258,6 +258,7 @@ qtests_aarch64 = \
> (config_all_accel.has_key('CONFIG_TCG') and \
> config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
> (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
> + qtests_cxl + \
> ['arm-cpu-features',
> 'numa-test',
> 'boot-serial-test',
> --
> 2.43.0
>
~/projects/qemu/build$ meson test qtest-aarch64/cxl-test
ninja: Entering directory `/home/realm/projects/qemu/build'
[1/8] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
1/1 qemu:qtest+qtest-aarch64 / qtest-aarch64/cxl-test OK 0.17s 1 subtests passed
Ok: 1
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com <mailto:itaru.kitayama@fujitsu.com>>
Jonathan, would you push your branch this series applied? I manually applied your series no issues though.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl
2025-05-13 11:14 [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Jonathan Cameron
` (4 preceding siblings ...)
2025-05-13 11:14 ` [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL Jonathan Cameron
@ 2025-05-16 2:30 ` Itaru Kitayama
2025-05-16 6:34 ` Itaru Kitayama
2025-05-20 17:31 ` Jonathan Cameron
5 siblings, 2 replies; 24+ messages in thread
From: Itaru Kitayama @ 2025-05-16 2:30 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
Hi Jonathan,
> On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>
> V13:
> - Make CXL fixed memory windows sysbus devices.
> IIRC this was requested by Peter in one of the reviews a long time back
> but at the time the motivation was less strong than it becomes with some
> WiP patches for hotness monitoring and high performance direct connect
> where we need a machine type independent way to iterate all the CXL
> fixed memory windows. This is a convenient place to do it so drag that
> work forward into this series.
>
> This allows us to drop separate list and necessary machine specific
> access code in favour of
> object_child_foreach_recursive(object_get_root(),...)
> One snag is that the ordering of multiple fixed memory windows in that
> walk depends on the underlying g_hash_table iterations rather than the
> order of creation. In the memory map layout and ACPI table creation we
> need both stable and predictable ordering. Resolve this in a similar
> fashion to object_class_get_list_sorted() be throwing them in a GSList
> and sorting that. Only use this when a sorted list is needed.
>
> Dropped RFC as now I'm happy with this code and would like to get it
> upstream! Particularly as it broken even today due to enscripten
> related changes that stop us using g_slist_sort(). Easy fix though.
>
> Note that we have an issue for CXL emulation in general and TCG which
> is being discussed in:
> https://lore.kernel.org/all/20250425183524.00000b28@huawei.com/
> (also affects some other platforms)
>
> Until that is resolved, either rebase this back on 10.0 or just
> don't let code run out of it (don't use KMEM to expose it as normal
> memory, use DAX instead).
>
> Previous cover letter.
>
> Back in 2022, this series stalled on the absence of a solution to device
> tree support for PCI Expander Bridges (PXB) and we ended up only having
> x86 support upstream. I've been carrying the arm64 support out of tree
> since then, with occasional nasty surprises (e.g. UNIMP + DT issue seen
> a few weeks ago) and a fair number of fiddly rebases.
> gitlab.com/jic23/qemu cxl-<latest date>
>
> A recent discussion with Peter Maydell indicated that there are various
> other ACPI only features now, so in general he might be more relaxed
> about DT support being necessary. The upcoming vSMMUv3 support would
> run into this problem as well.
>
> I presented the background to the PXB issue at Linaro connect 2022. In
> short the issue is that PXBs steal MMIO space from the main PCI root
> bridge. The challenge is knowing how much to steal.
>
> On ACPI platforms, we can rely on EDK2 to perform an enumeration and
> configuration of the PCI topology and QEMU can update the ACPI tables
> after EDK2 has done this when it can simply read the space used by the
> root ports. On device tree, there is no entity to figure out that
> enumeration so we don't know how to size the stolen region.
>
> Three approaches were discussed:
> 1) Enumerating in QEMU. Horribly complex and the last thing we want is a
> 3rd enumeration implementation that ends up out of sync with EDK2 and
> the kernel (there are frequent issues because of how those existing
> implementations differ.
> 2) Figure out how to enumerate in kernel. I never put a huge amount of work
> into this, but it seemed likely to involve a nasty dance with similar
> very specific code to that EDK2 is carrying and would very challenging
> to upstream (given the lack of clarity on real use cases for PXBs and
> DT).
> 3) Hack it based on the control we have which is bus numbers.
> No one liked this but it worked :)
>
> The other little wrinkle would be the need to define full bindings for CXL
> on DT + implement a fairly complex kernel stack as equivalent in ACPI
> involves a static table, CEDT, new runtime queries via _DSM and a description
> of various components. Doable, but so far there is no interest on physical
> platforms. Worth noting that for now, the QEMU CXL emulation is all about
> testing and developing the OS stack, not about virtualization (performance
> is terrible except in some very contrived situations!)
>
> Back to posting as an RFC because there was some discussion of approach to
> modelling the devices that may need a bit of redesign.
> The discussion kind of died out on the back of DT issue and I doubt anyone
> can remember the details.
>
> https://lore.kernel.org/qemu-devel/20220616141950.23374-1-Jonathan.Cameron@huawei.com/
>
> There is only a very simple test in here, because my intent is not to
> duplicate what we have on x86, but just to do a smoke test that everything
> is hooked up. In general we need much more comprehensive end to end CXL
> tests but that requires a reaonsably stable guest software stack. A few
> people have expressed interest in working on that, but we aren't there yet.
>
> Note that this series has a very different use case to that in the proposed
> SBSA-ref support:
> https://lore.kernel.org/qemu-devel/20250117034343.26356-1-wangyuquan1236@phytium.com.cn/
>
> SBSA-ref is a good choice if you want a relatively simple mostly fixed
> configuration. That works well with the limited host system
> discoverability etc as EDK2 can be build against a known configuration.
>
> My interest with this support in arm/virt is support host software stack
> development (we have a wide range of contributors, most of whom are working
> on emulation + the kernel support). I care about the weird corners. As such
> I need to be able to bring up variable numbers of host bridges, multiple CXL
> Fixed Memory Windows with varying characteristics (interleave etc), complex
> NUMA topologies with wierd performance characteristics etc. We can do that
> on x86 upstream today, or my gitlab tree. Note that we need arm support
> for some arch specific features in the near future (cache flushing).
> Doing kernel development with this need for flexibility on SBSA-ref is not
> currently practical. SBSA-ref CXL support is an excellent thing, just
> not much use to me for this work.
>
> Jonathan Cameron (5):
> hw/cxl-host: Add an index field to CXLFixedMemoryWindow
> hw/cxl: Make the CXL fixed memory windows devices.
> hw/cxl-host: Allow split of establishing memory address and mmio
> setup.
> hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances
> pxb-cxl
> qtest/cxl: Add aarch64 virt test for CXL
>
> include/hw/arm/virt.h | 4 +
> include/hw/cxl/cxl.h | 4 +
> include/hw/cxl/cxl_host.h | 6 +-
> hw/acpi/cxl.c | 83 +++++++++------
> hw/arm/virt-acpi-build.c | 34 ++++++
> hw/arm/virt.c | 29 +++++
> hw/cxl/cxl-host-stubs.c | 8 +-
> hw/cxl/cxl-host.c | 218 ++++++++++++++++++++++++++++++++------
> hw/i386/pc.c | 51 ++++-----
> tests/qtest/cxl-test.c | 59 ++++++++---
> tests/qtest/meson.build | 1 +
> 11 files changed, 389 insertions(+), 108 deletions(-)
>
> --
> 2.43.0
>
With your series applied on top of upstream QEMU, the -drive option does not work well with the sane CXL
setup (I use run_qemu.sh maintained by Marc et al. at Intel) see below:
/home/realm/projects/qemu/build/qemu-system-aarch64 -machine virt,accel=tcg,cxl=on,highmem=on,compact-highmem=on,highmem-ecam=on,highmem-mmio=on -m 2048M,slots=0,maxmem=6144M -smp 2,sockets=1,cores=2,threads=1 -display none -nographic -drive if=pflash,format=raw,unit=0,file=AAVMF_CODE.fd,readonly=on -drive if=pflash,format=raw,unit=1,file=AAVMF_VARS.fd -drive file=root.img,format=raw,media=disk -kernel mkosi.extra/boot/vmlinuz-6.15.0-rc4-00040-g128ad8fa385b -initrd mkosi.extra/boot/initramfs-6.15.0-rc4-00040-g128ad8fa385b.img -append selinux=0 audit=0 console=tty0 console=ttyS0 root=PARTUUID=14d6bae9-c917-435d-89ea-99af1fa4439a ignore_loglevel rw initcall_debug log_buf_len=20M memory_hotplug.memmap_on_memory=force cxl_acpi.dyndbg=+fplm cxl_pci.dyndbg=+fplm cxl_core.dyndbg=+fplm cxl_mem.dyndbg=+fplm cxl_pmem.dyndbg=+fplm cxl_port.dyndbg=+fplm cxl_region.dyndbg=+fplm cxl_test.dyndbg=+fplm cxl_mock.dyndbg=+fplm cxl_mock_mem.dyndbg=+fplm systemd.set_credential=agetty.autologin:root systemd.set_credential=login.noauth:yes -device e1000,netdev=net0,mac=52:54:00:12:34:56 -netdev user,id=net0,hostfwd=tcp::10022-:22 -cpu max -object memory-backend-file,id=cxl-mem0,share=on,mem-path=cxltest0.raw,size=256M -object memory-backend-file,id=cxl-mem1,share=on,mem-path=cxltest1.raw,size=256M -object memory-backend-file,id=cxl-mem2,share=on,mem-path=cxltest2.raw,size=256M -object memory-backend-file,id=cxl-mem3,share=on,mem-path=cxltest3.raw,size=256M -object memory-backend-file,id=cxl-lsa0,share=on,mem-path=lsa0.raw,size=128K -object memory-backend-file,id=cxl-lsa1,share=on,mem-path=lsa1.raw,size=128K -object memory-backend-file,id=cxl-lsa2,share=on,mem-path=lsa2.raw,size=128K -object memory-backend-file,id=cxl-lsa3,share=on,mem-path=lsa3.raw,size=128K -device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=53 -device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=191 -device cxl-rp,id=hb0rp0,bus=cxl.0,chassis=0,slot=0,port=0 -device cxl-rp,id=hb0rp1,bus=cxl.0,chassis=0,slot=1,port=1 -device cxl-rp,id=hb1rp0,bus=cxl.1,chassis=0,slot=2,port=0 -device cxl-rp,id=hb1rp1,bus=cxl.1,chassis=0,slot=3,port=1 -device cxl-upstream,port=4,bus=hb0rp0,id=cxl-up0,multifunction=on,addr=0.0,sn=12345678 -device cxl-switch-mailbox-cci,bus=hb0rp0,addr=0.1,target=cxl-up0 -device cxl-upstream,port=4,bus=hb1rp0,id=cxl-up1,multifunction=on,addr=0.0,sn=12341234 -device cxl-switch-mailbox-cci,bus=hb1rp0,addr=0.1,target=cxl-up1 -device cxl-downstream,port=0,bus=cxl-up0,id=swport0,chassis=0,slot=4 -device cxl-downstream,port=1,bus=cxl-up0,id=swport1,chassis=0,slot=5 -device cxl-downstream,port=2,bus=cxl-up0,id=swport2,chassis=0,slot=6 -device cxl-downstream,port=3,bus=cxl-up0,id=swport3,chassis=0,slot=7 -device cxl-downstream,port=0,bus=cxl-up1,id=swport4,chassis=0,slot=8 -device cxl-downstream,port=1,bus=cxl-up1,id=swport5,chassis=0,slot=9 -device cxl-downstream,port=2,bus=cxl-up1,id=swport6,chassis=0,slot=10 -device cxl-downstream,port=3,bus=cxl-up1,id=swport7,chassis=0,slot=11 -device cxl-type3,bus=swport0,persistent-memdev=cxl-mem0,id=cxl-pmem0,lsa=cxl-lsa0 -device cxl-type3,bus=swport2,persistent-memdev=cxl-mem1,id=cxl-pmem1,lsa=cxl-lsa1 -device cxl-type3,bus=swport4,volatile-memdev=cxl-mem2,id=cxl-vmem2,lsa=cxl-lsa2 -device cxl-type3,bus=swport6,volatile-memdev=cxl-mem3,id=cxl-vmem3,lsa=cxl-lsa3 -M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=8k,cxl-fmw.1.targets.0=cxl.0,cxl-fmw.1.targets.1=cxl.1,cxl-fmw.1.size=4G,cxl-fmw.1.interleave-granularity=8k -snapshot -object memory-backend-ram,id=mem0,size=2048M -numa node,nodeid=0,memdev=mem0, -numa cpu,node-id=0,socket-id=0 -numa dist,src=0,dst=0,val=10
qemu-system-aarch64: -drive file=root.img,format=raw,media=disk: PCI: Only PCI/PCIe bridges can be plugged into pxb-cxl
Plain upstream QEMU aarch64 target vert machine can handle the -drive option without an issue _without_ those cxl setup options added. I think the error was seen with your previous cxl-2025-03-20 branch.
Thanks,
Itaru.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices.
2025-05-13 11:14 ` [PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices Jonathan Cameron
@ 2025-05-16 5:44 ` Zhijian Li (Fujitsu)
2025-05-21 17:54 ` Fan Ni
2025-05-27 16:04 ` Jonathan Cameron
0 siblings, 2 replies; 24+ messages in thread
From: Zhijian Li (Fujitsu) @ 2025-05-16 5:44 UTC (permalink / raw)
To: Jonathan Cameron, qemu-devel@nongnu.org, Fan Ni, Peter Maydell,
mst@redhat.com
Cc: linux-cxl@vger.kernel.org, linuxarm@huawei.com,
qemu-arm@nongnu.org, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
On 13/05/2025 19:14, Jonathan Cameron via wrote:
> Previously these somewhat device like structures were tracked using a list
> in the CXLState in each machine. This is proving restrictive in a few
> cases where we need to iterate through these without being aware of the
> machine type. Just make them sysbus devices.
Make sense.
Minor comments inline
>
> Restrict them to not user created as they need to be visible to early
> stages of machine init given effects on the memory map.
>
> This change both simplifies state tracking and enables features needed
> for performance optimization and hotness tracking by making it possible
> to retrieve the fixed memory window on actions elsewhere in the topology.
>
> In some cases the ordering of the Fixed Memory Windows matters.
> For those utility functions provide a GSList sorted by the window index.
> This ensures that we get consistency across:
> - ordering in the command line
> - ordering of the host PA ranges
> - ordering of ACPI CEDT structures describing the CFMWS.
>
> Other aspects don't have this constraint. For those direct iteration
> of the underlying hash structures is fine.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> I think Peter Maydell suggested this a long time back when
> the original CXL support series was under review but not 100% sure.
> ---
> include/hw/cxl/cxl.h | 3 +
> include/hw/cxl/cxl_host.h | 4 +-
> hw/acpi/cxl.c | 83 +++++++++++--------
> hw/cxl/cxl-host-stubs.c | 6 +-
> hw/cxl/cxl-host.c | 169 +++++++++++++++++++++++++++++++-------
> hw/i386/pc.c | 51 ++++++------
> 6 files changed, 223 insertions(+), 93 deletions(-)
>
> diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> index b2bcce7ed6..a610795c87 100644
> --- a/include/hw/cxl/cxl.h
> +++ b/include/hw/cxl/cxl.h
> @@ -27,6 +27,7 @@
> typedef struct PXBCXLDev PXBCXLDev;
>
> typedef struct CXLFixedWindow {
> + SysBusDevice parent_obj;
> int index;
> uint64_t size;
> char **targets;
> @@ -38,6 +39,8 @@ typedef struct CXLFixedWindow {
> MemoryRegion mr;
> hwaddr base;
> } CXLFixedWindow;
> +#define TYPE_CXL_FMW "cxl-fmw"
> +OBJECT_DECLARE_SIMPLE_TYPE(CXLFixedWindow, CXL_FMW)
>
> typedef struct CXLState {
> bool is_enabled;
> diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
> index c9bc9c7c50..6dce2cde07 100644
> --- a/include/hw/cxl/cxl_host.h
> +++ b/include/hw/cxl/cxl_host.h
> @@ -14,8 +14,10 @@
> #define CXL_HOST_H
>
> void cxl_machine_init(Object *obj, CXLState *state);
> -void cxl_fmws_link_targets(CXLState *stat, Error **errp);
> +void cxl_fmws_link_targets(Error **errp);
> void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp);
> +hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr);
> +GSList *cxl_fmws_get_all_sorted(void);
>
> extern const MemoryRegionOps cfmws_ops;
>
> diff --git a/hw/acpi/cxl.c b/hw/acpi/cxl.c
> index 9cd7905ea2..20806e5dd4 100644
> --- a/hw/acpi/cxl.c
> +++ b/hw/acpi/cxl.c
> @@ -22,6 +22,7 @@
> #include "hw/pci/pci_bridge.h"
> #include "hw/pci/pci_host.h"
> #include "hw/cxl/cxl.h"
> +#include "hw/cxl/cxl_host.h"
> #include "hw/mem/memory-device.h"
> #include "hw/acpi/acpi.h"
> #include "hw/acpi/aml-build.h"
> @@ -135,56 +136,62 @@ static void cedt_build_chbs(GArray *table_data, PXBCXLDev *cxl)
> * Interleave ways encoding in CXL 2.0 ECN: 3, 6, 12 and 16-way memory
> * interleaving.
> */
> -static void cedt_build_cfmws(GArray *table_data, CXLState *cxls)
> +static int cedt_build_cfmws(Object *obj, void *opaque)
Too much unrelated indent updates in this function
> {
> - GList *it;
> + struct CXLFixedWindow *fw;
> + Aml *cedt = opaque;
> + GArray *table_data = cedt->buf;
> + int i;
>
> - for (it = cxls->fixed_windows; it; it = it->next) {
> - CXLFixedWindow *fw = it->data;
> - int i;
> + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> + return 0;
Never reach here? It seems the caller has ensured passing TYPE_CXL_FMW obj.
> + }
> + fw = CXL_FMW(obj);
>
> - /* Type */
> - build_append_int_noprefix(table_data, 1, 1);
> + /* Type */
> + build_append_int_noprefix(table_data, 1, 1);
>
> - /* Reserved */
> - build_append_int_noprefix(table_data, 0, 1);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 1);
>
> - /* Record Length */
> - build_append_int_noprefix(table_data, 36 + 4 * fw->num_targets, 2);
> + /* Record Length */
> + build_append_int_noprefix(table_data, 36 + 4 * fw->num_targets, 2);
>
> - /* Reserved */
> - build_append_int_noprefix(table_data, 0, 4);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 4);
>
> - /* Base HPA */
> - build_append_int_noprefix(table_data, fw->mr.addr, 8);
> + /* Base HPA */
> + build_append_int_noprefix(table_data, fw->mr.addr, 8);
>
> - /* Window Size */
> - build_append_int_noprefix(table_data, fw->size, 8);
> + /* Window Size */
> + build_append_int_noprefix(table_data, fw->size, 8);
>
> - /* Host Bridge Interleave Ways */
> - build_append_int_noprefix(table_data, fw->enc_int_ways, 1);
> + /* Host Bridge Interleave Ways */
> + build_append_int_noprefix(table_data, fw->enc_int_ways, 1);
>
> - /* Host Bridge Interleave Arithmetic */
> - build_append_int_noprefix(table_data, 0, 1);
> + /* Host Bridge Interleave Arithmetic */
> + build_append_int_noprefix(table_data, 0, 1);
>
> - /* Reserved */
> - build_append_int_noprefix(table_data, 0, 2);
> + /* Reserved */
> + build_append_int_noprefix(table_data, 0, 2);
>
> - /* Host Bridge Interleave Granularity */
> - build_append_int_noprefix(table_data, fw->enc_int_gran, 4);
> + /* Host Bridge Interleave Granularity */
> + build_append_int_noprefix(table_data, fw->enc_int_gran, 4);
>
> - /* Window Restrictions */
> - build_append_int_noprefix(table_data, 0x0f, 2); /* No restrictions */
> + /* Window Restrictions */
> + build_append_int_noprefix(table_data, 0x0f, 2);
>
> - /* QTG ID */
> - build_append_int_noprefix(table_data, 0, 2);
> + /* QTG ID */
> + build_append_int_noprefix(table_data, 0, 2);
>
> - /* Host Bridge List (list of UIDs - currently bus_nr) */
> - for (i = 0; i < fw->num_targets; i++) {
> - g_assert(fw->target_hbs[i]);
> - build_append_int_noprefix(table_data, PXB_DEV(fw->target_hbs[i])->bus_nr, 4);
> - }
> + /* Host Bridge List (list of UIDs - currently bus_nr) */
> + for (i = 0; i < fw->num_targets; i++) {
> + g_assert(fw->target_hbs[i]);
> + build_append_int_noprefix(table_data,
> + PXB_DEV(fw->target_hbs[i])->bus_nr, 4);
> }
> +
> + return 0;
> }
>
> static int cxl_foreach_pxb_hb(Object *obj, void *opaque)
> @@ -202,6 +209,7 @@ void cxl_build_cedt(GArray *table_offsets, GArray *table_data,
> BIOSLinker *linker, const char *oem_id,
> const char *oem_table_id, CXLState *cxl_state)
> {
> + GSList *cfmws_list, *iter;
> Aml *cedt;
> AcpiTable table = { .sig = "CEDT", .rev = 1, .oem_id = oem_id,
> .oem_table_id = oem_table_id };
> @@ -213,7 +221,12 @@ void cxl_build_cedt(GArray *table_offsets, GArray *table_data,
> /* reserve space for CEDT header */
>
> object_child_foreach_recursive(object_get_root(), cxl_foreach_pxb_hb, cedt);
> - cedt_build_cfmws(cedt->buf, cxl_state);
> +
> + cfmws_list = cxl_fmws_get_all_sorted();
> + for (iter = cfmws_list; iter; iter = iter->next) {
> + cedt_build_cfmws(iter->data, cedt);
> + }
> + g_slist_free(cfmws_list);
>
> /* copy AML table into ACPI tables blob and patch header there */
> g_array_append_vals(table_data, cedt->buf->data, cedt->buf->len);
> diff --git a/hw/cxl/cxl-host-stubs.c b/hw/cxl/cxl-host-stubs.c
> index cae4afcdde..13eb6bf6a4 100644
> --- a/hw/cxl/cxl-host-stubs.c
> +++ b/hw/cxl/cxl-host-stubs.c
> @@ -8,8 +8,12 @@
> #include "hw/cxl/cxl.h"
> #include "hw/cxl/cxl_host.h"
>
> -void cxl_fmws_link_targets(CXLState *stat, Error **errp) {};
> +void cxl_fmws_link_targets(Error **errp) {};
> void cxl_machine_init(Object *obj, CXLState *state) {};
> void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp) {};
> +hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> +{
> + return base;
> +};
>
> const MemoryRegionOps cfmws_ops;
> diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> index b7aa429ddf..438f2920e1 100644
> --- a/hw/cxl/cxl-host.c
> +++ b/hw/cxl/cxl-host.c
> @@ -22,12 +22,12 @@
> #include "hw/pci/pcie_port.h"
> #include "hw/pci-bridge/pci_expander_bridge.h"
>
> -static void cxl_fixed_memory_window_config(CXLState *cxl_state,
> - CXLFixedMemoryWindowOptions *object,
> +static void cxl_fixed_memory_window_config(CXLFixedMemoryWindowOptions *object,
> int index, Error **errp)
> {
> ERRP_GUARD();
> - g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
> + DeviceState *dev = qdev_new(TYPE_CXL_FMW);
> + CXLFixedWindow *fw = CXL_FMW(dev);
> strList *target;
> int i;
>
> @@ -67,35 +67,41 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state,
> fw->targets[i] = g_strdup(target->value);
> }
>
> - cxl_state->fixed_windows = g_list_append(cxl_state->fixed_windows,
> - g_steal_pointer(&fw));
> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp);
> +
> + return;
> }
>
> -void cxl_fmws_link_targets(CXLState *cxl_state, Error **errp)
> +static int cxl_fmws_link(Object *obj, void *opaque)
> {
> - if (cxl_state && cxl_state->fixed_windows) {
> - GList *it;
> -
> - for (it = cxl_state->fixed_windows; it; it = it->next) {
> - CXLFixedWindow *fw = it->data;
> - int i;
> -
> - for (i = 0; i < fw->num_targets; i++) {
> - Object *o;
> - bool ambig;
> -
> - o = object_resolve_path_type(fw->targets[i],
> - TYPE_PXB_CXL_DEV,
> - &ambig);
> - if (!o) {
> - error_setg(errp, "Could not resolve CXLFM target %s",
> - fw->targets[i]);
> - return;
> - }
> - fw->target_hbs[i] = PXB_CXL_DEV(o);
> - }
> + struct CXLFixedWindow *fw;
> + int i;
> +
> + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> + return 0;
> + }
> + fw = CXL_FMW(obj);
> +
> + for (i = 0; i < fw->num_targets; i++) {
> + Object *o;
> + bool ambig;
> +
> + o = object_resolve_path_type(fw->targets[i], TYPE_PXB_CXL_DEV,
> + &ambig);
> + if (!o) {
> + error_setg(&error_fatal, "Could not resolve CXLFM target %s",
> + fw->targets[i]);
> + return 1;
> }
> + fw->target_hbs[i] = PXB_CXL_DEV(o);
> }
> + return 0;
> +}
> +
> +void cxl_fmws_link_targets(Error **errp)
> +{
> + /* Order doesn't matter for this, so no need to build list */
> + object_child_foreach_recursive(object_get_root(), cxl_fmws_link, NULL);
> }
>
> static bool cxl_hdm_find_target(uint32_t *cache_mem, hwaddr addr,
> @@ -335,7 +341,7 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
> }
>
> for (it = cfmw_list, index = 0; it; it = it->next, index++) {
> - cxl_fixed_memory_window_config(state, it->value, index, errp);
> + cxl_fixed_memory_window_config(it->value, index, errp);
> }
> state->cfmw_list = cfmw_list;
> }
> @@ -373,3 +379,110 @@ void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp)
> }
> }
> }
> +
> +struct cfmw_update_state {
> + hwaddr base;
> + hwaddr maxaddr;
> +};
> +
> +static void cxl_fmws_update(Object *obj, void *opaque)
> +{
> + struct CXLFixedWindow *fw;
> + struct cfmw_update_state *s = opaque;
> +
> + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
Never reach here? It seems the caller has ensured passing TYPE_CXL_FMW obj.
Thanks
Zhijian
> + return;
> + }
> +
> + fw = CXL_FMW(obj);
> + if (s->base + fw->size <= s->maxaddr) {
> + fw->base = s->base;
> + sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
> + s->base += fw->size;
> + }
> +
> + return;
> +}
> +
> +static int cxl_fmws_find(Object *obj, void *opaque)
> +{
> + GSList **list = opaque;
> +
> + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> + return 0;
> + }
> + *list = g_slist_prepend(*list, obj);
> +
> + return 0;
> +}
> +
> +static GSList *cxl_fmws_get_all(void)
> +{
> + GSList *list = NULL;
> +
> + object_child_foreach_recursive(object_get_root(), cxl_fmws_find, &list);
> +
> + return list;
> +}
> +
> +static gint cfmws_cmp(gconstpointer a, gconstpointer b, gpointer d)
> +{
> + const struct CXLFixedWindow *ap = a;
> + const struct CXLFixedWindow *bp = b;
> +
> + return ap->index > bp->index;
> +}
> +
> +GSList *cxl_fmws_get_all_sorted(void)
> +{
> + return g_slist_sort_with_data(cxl_fmws_get_all(), cfmws_cmp, NULL);
> +}
> +
> +hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> +{
> + GSList *cfmws_list, *iter;
> +
> + struct cfmw_update_state cfmwss = {
> + .base = base,
> + .maxaddr = max_addr,
> + };
> + cfmws_list = cxl_fmws_get_all_sorted();
> + for (iter = cfmws_list; iter; iter = iter->next) {
> + cxl_fmws_update(iter->data, &cfmwss);
> + }
> + g_slist_free(cfmws_list);
> +
> + return cfmwss.base;
> +}
> +
> +static void cxl_fmw_realize(DeviceState *dev, Error **errp)
> +{
> + CXLFixedWindow *fw = CXL_FMW(dev);
> +
> + memory_region_init_io(&fw->mr, OBJECT(dev), &cfmws_ops, fw,
> + "cxl-fixed-memory-region", fw->size);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &fw->mr);
> +}
> +
> +static void cxl_fmw_class_init(ObjectClass *klass, const void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->desc = "CXL Fixed Memory Window";
> + dc->realize = cxl_fmw_realize;
> + /* Reason - created by machines as tightly coupled to machine memory map */
> + dc->user_creatable = false;
> +}
> +
> +static const TypeInfo cxl_fmw_info = {
> + .name = TYPE_CXL_FMW,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(CXLFixedWindow),
> + .class_init = cxl_fmw_class_init,
> +};
> +
> +static void cxl_host_register_types(void)
> +{
> + type_register_static(&cxl_fmw_info);
> +}
> +type_init(cxl_host_register_types)
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 70656157ca..9978398876 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -630,7 +630,7 @@ void pc_machine_done(Notifier *notifier, void *data)
> &error_fatal);
>
> if (pcms->cxl_devices_state.is_enabled) {
> - cxl_fmws_link_targets(&pcms->cxl_devices_state, &error_fatal);
> + cxl_fmws_link_targets(&error_fatal);
> }
>
> /* set the number of CPUs */
> @@ -739,20 +739,28 @@ static uint64_t pc_get_cxl_range_start(PCMachineState *pcms)
> return cxl_base;
> }
>
> -static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
> +static int cxl_get_fmw_end(Object *obj, void *opaque)
> {
> - uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
> + struct CXLFixedWindow *fw;
> + uint64_t *start = opaque;
>
> - if (pcms->cxl_devices_state.fixed_windows) {
> - GList *it;
> -
> - start = ROUND_UP(start, 256 * MiB);
> - for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
> - CXLFixedWindow *fw = it->data;
> - start += fw->size;
> - }
> + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> + return 0;
> }
> + fw = CXL_FMW(obj);
> +
> + *start += fw->size;
>
> + return 0;
> +}
> +
> +static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
> +{
> + uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
> +
> + /* Ordering doesn't matter so no need to build a sorted list */
> + object_child_foreach_recursive(object_get_root(), cxl_get_fmw_end,
> + &start);
> return start;
> }
>
> @@ -954,23 +962,10 @@ void pc_memory_init(PCMachineState *pcms,
> cxl_base = pc_get_cxl_range_start(pcms);
> memory_region_init(mr, OBJECT(machine), "cxl_host_reg", cxl_size);
> memory_region_add_subregion(system_memory, cxl_base, mr);
> - cxl_resv_end = cxl_base + cxl_size;
> - if (pcms->cxl_devices_state.fixed_windows) {
> - hwaddr cxl_fmw_base;
> - GList *it;
> -
> - cxl_fmw_base = ROUND_UP(cxl_base + cxl_size, 256 * MiB);
> - for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
> - CXLFixedWindow *fw = it->data;
> -
> - fw->base = cxl_fmw_base;
> - memory_region_init_io(&fw->mr, OBJECT(machine), &cfmws_ops, fw,
> - "cxl-fixed-memory-region", fw->size);
> - memory_region_add_subregion(system_memory, fw->base, &fw->mr);
> - cxl_fmw_base += fw->size;
> - cxl_resv_end = cxl_fmw_base;
> - }
> - }
> + cxl_base = ROUND_UP(cxl_base + cxl_size, 256 * MiB);
> +
> + cxl_resv_end = cxl_fmws_set_memmap_and_update_mmio(cxl_base,
> + maxphysaddr);
> }
>
> /* Initialize PC system firmware */
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow
2025-05-13 11:14 ` [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow Jonathan Cameron
@ 2025-05-16 5:44 ` Zhijian Li (Fujitsu)
2025-05-21 16:59 ` Fan Ni
` (2 subsequent siblings)
3 siblings, 0 replies; 24+ messages in thread
From: Zhijian Li (Fujitsu) @ 2025-05-16 5:44 UTC (permalink / raw)
To: Jonathan Cameron, qemu-devel@nongnu.org, Fan Ni, Peter Maydell,
mst@redhat.com
Cc: linux-cxl@vger.kernel.org, linuxarm@huawei.com,
qemu-arm@nongnu.org, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
On 13/05/2025 19:14, Jonathan Cameron via wrote:
> To enable these to be found in a fixed order, that order needs
> to be known. This will later be used to sort a list of these
> structures that address map and ACPI table entries are predictable.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> include/hw/cxl/cxl.h | 1 +
> hw/cxl/cxl-host.c | 9 ++++++---
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> index 75e47b6864..b2bcce7ed6 100644
> --- a/include/hw/cxl/cxl.h
> +++ b/include/hw/cxl/cxl.h
> @@ -27,6 +27,7 @@
> typedef struct PXBCXLDev PXBCXLDev;
>
> typedef struct CXLFixedWindow {
> + int index;
> uint64_t size;
> char **targets;
> PXBCXLDev *target_hbs[16];
> diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> index e010163174..b7aa429ddf 100644
> --- a/hw/cxl/cxl-host.c
> +++ b/hw/cxl/cxl-host.c
> @@ -24,13 +24,15 @@
>
> static void cxl_fixed_memory_window_config(CXLState *cxl_state,
> CXLFixedMemoryWindowOptions *object,
> - Error **errp)
> + int index, Error **errp)
> {
> ERRP_GUARD();
> g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
> strList *target;
> int i;
>
> + fw->index = index;
> +
> for (target = object->targets; target; target = target->next) {
> fw->num_targets++;
> }
> @@ -325,14 +327,15 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
> CXLState *state = opaque;
> CXLFixedMemoryWindowOptionsList *cfmw_list = NULL;
> CXLFixedMemoryWindowOptionsList *it;
> + int index;
>
> visit_type_CXLFixedMemoryWindowOptionsList(v, name, &cfmw_list, errp);
> if (!cfmw_list) {
> return;
> }
>
> - for (it = cfmw_list; it; it = it->next) {
> - cxl_fixed_memory_window_config(state, it->value, errp);
> + for (it = cfmw_list, index = 0; it; it = it->next, index++) {
> + cxl_fixed_memory_window_config(state, it->value, index, errp);
> }
> state->cfmw_list = cfmw_list;
> }
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup.
2025-05-13 11:14 ` [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup Jonathan Cameron
@ 2025-05-16 5:50 ` Zhijian Li (Fujitsu)
2025-05-27 16:28 ` Jonathan Cameron
0 siblings, 1 reply; 24+ messages in thread
From: Zhijian Li (Fujitsu) @ 2025-05-16 5:50 UTC (permalink / raw)
To: Jonathan Cameron, qemu-devel@nongnu.org, Fan Ni, Peter Maydell,
mst@redhat.com
Cc: linux-cxl@vger.kernel.org, linuxarm@huawei.com,
qemu-arm@nongnu.org, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
On 13/05/2025 19:14, Jonathan Cameron via wrote:
>
> +hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr)
> +{
> + GSList *cfmws_list, *iter;
> +
> + struct cfmw_update_state cfmwss = {
> + .base = base,
> + .maxaddr = max_addr,
> + .update_mmio = false,
> + };
> + cfmws_list = cxl_fmws_get_all_sorted();
> + for (iter = cfmws_list; iter; iter = iter->next) {
> + cxl_fmws_update(iter->data, &cfmwss);
> + }
> + g_slist_free(cfmws_list);
> +
> + return cfmwss.base;
> +}
> +
> hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> {
> GSList *cfmws_list, *iter;
> @@ -445,6 +466,7 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> struct cfmw_update_state cfmwss = {
> .base = base,
> .maxaddr = max_addr,
> + .update_mmio = true,
> };
> cfmws_list = cxl_fmws_get_all_sorted();
> for (iter = cfmws_list; iter; iter = iter->next) {
> @@ -455,6 +477,26 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> return cfmwss.base;
> }
>
It seem we can share most of the code in cxl_fmws_set_memmap_and_update_mmio() and cxl_fmws_set_memmap()
In addition, we can drop the cfmw_update_state::update_mmio if there is no other users. Just pass it
to cxl_fmws_update() directly.
Thanks
Zhijian
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl
2025-05-16 2:30 ` [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Itaru Kitayama
@ 2025-05-16 6:34 ` Itaru Kitayama
2025-05-20 17:31 ` Jonathan Cameron
1 sibling, 0 replies; 24+ messages in thread
From: Itaru Kitayama @ 2025-05-16 6:34 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
> On May 16, 2025, at 11:30, Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
>
> Hi Jonathan,
>
>> On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>>
>> V13:
>> - Make CXL fixed memory windows sysbus devices.
>> IIRC this was requested by Peter in one of the reviews a long time back
>> but at the time the motivation was less strong than it becomes with some
>> WiP patches for hotness monitoring and high performance direct connect
>> where we need a machine type independent way to iterate all the CXL
>> fixed memory windows. This is a convenient place to do it so drag that
>> work forward into this series.
>>
>> This allows us to drop separate list and necessary machine specific
>> access code in favour of
>> object_child_foreach_recursive(object_get_root(),...)
>> One snag is that the ordering of multiple fixed memory windows in that
>> walk depends on the underlying g_hash_table iterations rather than the
>> order of creation. In the memory map layout and ACPI table creation we
>> need both stable and predictable ordering. Resolve this in a similar
>> fashion to object_class_get_list_sorted() be throwing them in a GSList
>> and sorting that. Only use this when a sorted list is needed.
>>
>> Dropped RFC as now I'm happy with this code and would like to get it
>> upstream! Particularly as it broken even today due to enscripten
>> related changes that stop us using g_slist_sort(). Easy fix though.
>>
>> Note that we have an issue for CXL emulation in general and TCG which
>> is being discussed in:
>> https://lore.kernel.org/all/20250425183524.00000b28@huawei.com/
>> (also affects some other platforms)
>>
>> Until that is resolved, either rebase this back on 10.0 or just
>> don't let code run out of it (don't use KMEM to expose it as normal
>> memory, use DAX instead).
>>
>> Previous cover letter.
>>
>> Back in 2022, this series stalled on the absence of a solution to device
>> tree support for PCI Expander Bridges (PXB) and we ended up only having
>> x86 support upstream. I've been carrying the arm64 support out of tree
>> since then, with occasional nasty surprises (e.g. UNIMP + DT issue seen
>> a few weeks ago) and a fair number of fiddly rebases.
>> gitlab.com/jic23/qemu cxl-<latest date>
>>
>> A recent discussion with Peter Maydell indicated that there are various
>> other ACPI only features now, so in general he might be more relaxed
>> about DT support being necessary. The upcoming vSMMUv3 support would
>> run into this problem as well.
>>
>> I presented the background to the PXB issue at Linaro connect 2022. In
>> short the issue is that PXBs steal MMIO space from the main PCI root
>> bridge. The challenge is knowing how much to steal.
>>
>> On ACPI platforms, we can rely on EDK2 to perform an enumeration and
>> configuration of the PCI topology and QEMU can update the ACPI tables
>> after EDK2 has done this when it can simply read the space used by the
>> root ports. On device tree, there is no entity to figure out that
>> enumeration so we don't know how to size the stolen region.
>>
>> Three approaches were discussed:
>> 1) Enumerating in QEMU. Horribly complex and the last thing we want is a
>> 3rd enumeration implementation that ends up out of sync with EDK2 and
>> the kernel (there are frequent issues because of how those existing
>> implementations differ.
>> 2) Figure out how to enumerate in kernel. I never put a huge amount of work
>> into this, but it seemed likely to involve a nasty dance with similar
>> very specific code to that EDK2 is carrying and would very challenging
>> to upstream (given the lack of clarity on real use cases for PXBs and
>> DT).
>> 3) Hack it based on the control we have which is bus numbers.
>> No one liked this but it worked :)
>>
>> The other little wrinkle would be the need to define full bindings for CXL
>> on DT + implement a fairly complex kernel stack as equivalent in ACPI
>> involves a static table, CEDT, new runtime queries via _DSM and a description
>> of various components. Doable, but so far there is no interest on physical
>> platforms. Worth noting that for now, the QEMU CXL emulation is all about
>> testing and developing the OS stack, not about virtualization (performance
>> is terrible except in some very contrived situations!)
>>
>> Back to posting as an RFC because there was some discussion of approach to
>> modelling the devices that may need a bit of redesign.
>> The discussion kind of died out on the back of DT issue and I doubt anyone
>> can remember the details.
>>
>> https://lore.kernel.org/qemu-devel/20220616141950.23374-1-Jonathan.Cameron@huawei.com/
>>
>> There is only a very simple test in here, because my intent is not to
>> duplicate what we have on x86, but just to do a smoke test that everything
>> is hooked up. In general we need much more comprehensive end to end CXL
>> tests but that requires a reaonsably stable guest software stack. A few
>> people have expressed interest in working on that, but we aren't there yet.
>>
>> Note that this series has a very different use case to that in the proposed
>> SBSA-ref support:
>> https://lore.kernel.org/qemu-devel/20250117034343.26356-1-wangyuquan1236@phytium.com.cn/
>>
>> SBSA-ref is a good choice if you want a relatively simple mostly fixed
>> configuration. That works well with the limited host system
>> discoverability etc as EDK2 can be build against a known configuration.
>>
>> My interest with this support in arm/virt is support host software stack
>> development (we have a wide range of contributors, most of whom are working
>> on emulation + the kernel support). I care about the weird corners. As such
>> I need to be able to bring up variable numbers of host bridges, multiple CXL
>> Fixed Memory Windows with varying characteristics (interleave etc), complex
>> NUMA topologies with wierd performance characteristics etc. We can do that
>> on x86 upstream today, or my gitlab tree. Note that we need arm support
>> for some arch specific features in the near future (cache flushing).
>> Doing kernel development with this need for flexibility on SBSA-ref is not
>> currently practical. SBSA-ref CXL support is an excellent thing, just
>> not much use to me for this work.
>>
>> Jonathan Cameron (5):
>> hw/cxl-host: Add an index field to CXLFixedMemoryWindow
>> hw/cxl: Make the CXL fixed memory windows devices.
>> hw/cxl-host: Allow split of establishing memory address and mmio
>> setup.
>> hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances
>> pxb-cxl
>> qtest/cxl: Add aarch64 virt test for CXL
>>
>> include/hw/arm/virt.h | 4 +
>> include/hw/cxl/cxl.h | 4 +
>> include/hw/cxl/cxl_host.h | 6 +-
>> hw/acpi/cxl.c | 83 +++++++++------
>> hw/arm/virt-acpi-build.c | 34 ++++++
>> hw/arm/virt.c | 29 +++++
>> hw/cxl/cxl-host-stubs.c | 8 +-
>> hw/cxl/cxl-host.c | 218 ++++++++++++++++++++++++++++++++------
>> hw/i386/pc.c | 51 ++++-----
>> tests/qtest/cxl-test.c | 59 ++++++++---
>> tests/qtest/meson.build | 1 +
>> 11 files changed, 389 insertions(+), 108 deletions(-)
>>
>> --
>> 2.43.0
>>
>
> With your series applied on top of upstream QEMU, the -drive option does not work well with the sane CXL
> setup (I use run_qemu.sh maintained by Marc et al. at Intel) see below:
>
> /home/realm/projects/qemu/build/qemu-system-aarch64 -machine virt,accel=tcg,cxl=on,highmem=on,compact-highmem=on,highmem-ecam=on,highmem-mmio=on -m 2048M,slots=0,maxmem=6144M -smp 2,sockets=1,cores=2,threads=1 -display none -nographic -drive if=pflash,format=raw,unit=0,file=AAVMF_CODE.fd,readonly=on -drive if=pflash,format=raw,unit=1,file=AAVMF_VARS.fd -drive file=root.img,format=raw,media=disk -kernel mkosi.extra/boot/vmlinuz-6.15.0-rc4-00040-g128ad8fa385b -initrd mkosi.extra/boot/initramfs-6.15.0-rc4-00040-g128ad8fa385b.img -append selinux=0 audit=0 console=tty0 console=ttyS0 root=PARTUUID=14d6bae9-c917-435d-89ea-99af1fa4439a ignore_loglevel rw initcall_debug log_buf_len=20M memory_hotplug.memmap_on_memory=force cxl_acpi.dyndbg=+fplm cxl_pci.dyndbg=+fplm cxl_core.dyndbg=+fplm cxl_mem.dyndbg=+fplm cxl_pmem.dyndbg=+fplm cxl_port.dyndbg=+fplm cxl_region.dyndbg=+fplm cxl_test.dyndbg=+fplm cxl_mock.dyndbg=+fplm cxl_mock_mem.dyndbg=+fplm systemd.set_credential=agetty.autologin:root systemd.set_credential=login.noauth:yes -device e1000,netdev=net0,mac=52:54:00:12:34:56 -netdev user,id=net0,hostfwd=tcp::10022-:22 -cpu max -object memory-backend-file,id=cxl-mem0,share=on,mem-path=cxltest0.raw,size=256M -object memory-backend-file,id=cxl-mem1,share=on,mem-path=cxltest1.raw,size=256M -object memory-backend-file,id=cxl-mem2,share=on,mem-path=cxltest2.raw,size=256M -object memory-backend-file,id=cxl-mem3,share=on,mem-path=cxltest3.raw,size=256M -object memory-backend-file,id=cxl-lsa0,share=on,mem-path=lsa0.raw,size=128K -object memory-backend-file,id=cxl-lsa1,share=on,mem-path=lsa1.raw,size=128K -object memory-backend-file,id=cxl-lsa2,share=on,mem-path=lsa2.raw,size=128K -object memory-backend-file,id=cxl-lsa3,share=on,mem-path=lsa3.raw,size=128K -device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=53 -device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=191 -device cxl-rp,id=hb0rp0,bus=cxl.0,chassis=0,slot=0,port=0 -device cxl-rp,id=hb0rp1,bus=cxl.0,chassis=0,slot=1,port=1 -device cxl-rp,id=hb1rp0,bus=cxl.1,chassis=0,slot=2,port=0 -device cxl-rp,id=hb1rp1,bus=cxl.1,chassis=0,slot=3,port=1 -device cxl-upstream,port=4,bus=hb0rp0,id=cxl-up0,multifunction=on,addr=0.0,sn=12345678 -device cxl-switch-mailbox-cci,bus=hb0rp0,addr=0.1,target=cxl-up0 -device cxl-upstream,port=4,bus=hb1rp0,id=cxl-up1,multifunction=on,addr=0.0,sn=12341234 -device cxl-switch-mailbox-cci,bus=hb1rp0,addr=0.1,target=cxl-up1 -device cxl-downstream,port=0,bus=cxl-up0,id=swport0,chassis=0,slot=4 -device cxl-downstream,port=1,bus=cxl-up0,id=swport1,chassis=0,slot=5 -device cxl-downstream,port=2,bus=cxl-up0,id=swport2,chassis=0,slot=6 -device cxl-downstream,port=3,bus=cxl-up0,id=swport3,chassis=0,slot=7 -device cxl-downstream,port=0,bus=cxl-up1,id=swport4,chassis=0,slot=8 -device cxl-downstream,port=1,bus=cxl-up1,id=swport5,chassis=0,slot=9 -device cxl-downstream,port=2,bus=cxl-up1,id=swport6,chassis=0,slot=10 -device cxl-downstream,port=3,bus=cxl-up1,id=swport7,chassis=0,slot=11 -device cxl-type3,bus=swport0,persistent-memdev=cxl-mem0,id=cxl-pmem0,lsa=cxl-lsa0 -device cxl-type3,bus=swport2,persistent-memdev=cxl-mem1,id=cxl-pmem1,lsa=cxl-lsa1 -device cxl-type3,bus=swport4,volatile-memdev=cxl-mem2,id=cxl-vmem2,lsa=cxl-lsa2 -device cxl-type3,bus=swport6,volatile-memdev=cxl-mem3,id=cxl-vmem3,lsa=cxl-lsa3 -M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=8k,cxl-fmw.1.targets.0=cxl.0,cxl-fmw.1.targets.1=cxl.1,cxl-fmw.1.size=4G,cxl-fmw.1.interleave-granularity=8k -snapshot -object memory-backend-ram,id=mem0,size=2048M -numa node,nodeid=0,memdev=mem0, -numa cpu,node-id=0,socket-id=0 -numa dist,src=0,dst=0,val=10
> qemu-system-aarch64: -drive file=root.img,format=raw,media=disk: PCI: Only PCI/PCIe bridges can be plugged into pxb-cxl
>
> Plain upstream QEMU aarch64 target vert machine can handle the -drive option without an issue _without_ those cxl setup options added. I think the error was seen with your previous cxl-2025-03-20 branch.
>
> Thanks,
> Itaru.
While the above is not a show stopper for testing CXL, on the aarch64 target virt machine I get still errors:
[…]
22/48 ndctl:cxl / cxl-topology.sh FAIL 1.06s exit status 1
>>> NDCTL=/root/ndctl/build/ndctl/ndctl ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MALLOC_PERTURB_=66 MESON_TEST_ITERATION=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 LD_LIBRARY_PATH=/root/ndctl/build/ndctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/daxctl/lib DATA_PATH=/root/ndctl/test TEST_PATH=/root/ndctl/build/test DAXCTL=/root/ndctl/build/daxctl/daxctl UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /bin/bash /root/ndctl/test/cxl-topology.sh
23/48 ndctl:cxl / cxl-region-sysfs.sh FAIL 1.33s exit status 1
>>> NDCTL=/root/ndctl/build/ndctl/ndctl ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 LD_LIBRARY_PATH=/root/ndctl/build/ndctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/daxctl/lib DATA_PATH=/root/ndctl/test MALLOC_PERTURB_=252 TEST_PATH=/root/ndctl/build/test DAXCTL=/root/ndctl/build/daxctl/daxctl UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /bin/bash /root/ndctl/test/cxl-region-sysfs.sh
24/48 ndctl:cxl / cxl-labels.sh OK 2.44s
25/48 ndctl:cxl / cxl-create-region.sh FAIL 1.09s exit status 1
>>> NDCTL=/root/ndctl/build/ndctl/ndctl ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=1 MALLOC_PERTURB_=216 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 LD_LIBRARY_PATH=/root/ndctl/build/ndctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/daxctl/lib DATA_PATH=/root/ndctl/test TEST_PATH=/root/ndctl/build/test DAXCTL=/root/ndctl/build/daxctl/daxctl UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /bin/bash /root/ndctl/test/cxl-create-region.sh
26/48 ndctl:cxl / cxl-xor-region.sh SKIP 0.72s exit status 77
27/48 ndctl:cxl / cxl-events.sh FAIL 1.11s exit status 1
>>> NDCTL=/root/ndctl/build/ndctl/ndctl ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 LD_LIBRARY_PATH=/root/ndctl/build/ndctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/daxctl/lib DATA_PATH=/root/ndctl/test MALLOC_PERTURB_=4 TEST_PATH=/root/ndctl/build/test DAXCTL=/root/ndctl/build/daxctl/daxctl UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /bin/bash /root/ndctl/test/cxl-events.sh
28/48 ndctl:cxl / cxl-sanitize.sh FAIL 1.19s exit status 1
>>> NDCTL=/root/ndctl/build/ndctl/ndctl ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 LD_LIBRARY_PATH=/root/ndctl/build/ndctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/daxctl/lib DATA_PATH=/root/ndctl/test MALLOC_PERTURB_=103 TEST_PATH=/root/ndctl/build/test DAXCTL=/root/ndctl/build/daxctl/daxctl UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /bin/bash /root/ndctl/test/cxl-sanitize.sh
29/48 ndctl:cxl / cxl-destroy-region.sh OK 2.38s
30/48 ndctl:cxl / cxl-qos-class.sh FAIL 1.69s exit status 1
>>> NDCTL=/root/ndctl/build/ndctl/ndctl ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 LD_LIBRARY_PATH=/root/ndctl/build/ndctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/daxctl/lib DATA_PATH=/root/ndctl/test MALLOC_PERTURB_=118 TEST_PATH=/root/ndctl/build/test DAXCTL=/root/ndctl/build/daxctl/daxctl UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /root/ndctl/test/cxl-qos-class.sh
31/48 ndctl:cxl / cxl-poison.sh FAIL 0.71s exit status 1
>>> NDCTL=/root/ndctl/build/ndctl/ndctl ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MESON_TEST_ITERATION=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 LD_LIBRARY_PATH=/root/ndctl/build/ndctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/daxctl/lib DATA_PATH=/root/ndctl/test MALLOC_PERTURB_=174 TEST_PATH=/root/ndctl/build/test DAXCTL=/root/ndctl/build/daxctl/daxctl UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /bin/bash /root/ndctl/test/cxl-poison.sh
[…]
I’ll check there are due to allocation failure from the Host Physical Address space.
Itaru.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL
2025-05-15 9:04 ` Itaru Kitayama
@ 2025-05-19 12:54 ` Jonathan Cameron
2025-05-20 20:38 ` Itaru Kitayama
2025-05-21 7:38 ` Itaru Kitayama
0 siblings, 2 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-19 12:54 UTC (permalink / raw)
To: Itaru Kitayama
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
On Thu, 15 May 2025 18:04:18 +0900
Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
> > On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> >
> > Add a single complex case for aarch64 virt machine.
> > Given existing much more comprehensive tests for x86 cover the
> > common functionality, a single test should be enough to verify
> > that the aarch64 part continue to work.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > tests/qtest/cxl-test.c | 59 ++++++++++++++++++++++++++++++++---------
> > tests/qtest/meson.build | 1 +
> > 2 files changed, 47 insertions(+), 13 deletions(-)
> >
> > diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
> > index a600331843..c7189d6222 100644
> > --- a/tests/qtest/cxl-test.c
> > +++ b/tests/qtest/cxl-test.c
> > @@ -19,6 +19,12 @@
> > "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
> > "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
> >
> > +#define QEMU_VIRT_2PXB_CMD \
> > + "-machine virt,cxl=on -cpu max " \
> > + "-device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52 " \
> > + "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
> > + "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
> > +
> > #define QEMU_RP \
> > "-device cxl-rp,id=rp0,bus=cxl.0,chassis=0,slot=0 "
> >
> > @@ -197,25 +203,52 @@ static void cxl_2pxb_4rp_4t3d(void)
> > qtest_end();
> > rmdir(tmpfs);
> > }
> > +
> > +static void cxl_virt_2pxb_4rp_4t3d(void)
> > +{
> > + g_autoptr(GString) cmdline = g_string_new(NULL);
> > + char template[] = "/tmp/cxl-test-XXXXXX";
> > + const char *tmpfs;
> > +
> > + tmpfs = mkdtemp(template);
> > +
> > + g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
> > + tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
> > + tmpfs, tmpfs);
> > +
> > + qtest_start(cmdline->str);
> > + qtest_end();
> > + rmdir(tmpfs);
> > +}
> > #endif /* CONFIG_POSIX */
> >
> > int main(int argc, char **argv)
> > {
> > - g_test_init(&argc, &argv, NULL);
> > + const char *arch = qtest_get_arch();
> >
> > - qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
> > - qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
> > - qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
> > - qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
> > - qtest_add_func("/pci/cxl/rp", cxl_root_port);
> > - qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
> > + g_test_init(&argc, &argv, NULL);
> > + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> > + qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
> > + qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
> > + qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
> > + qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
> > + qtest_add_func("/pci/cxl/rp", cxl_root_port);
> > + qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
> > #ifdef CONFIG_POSIX
> > - qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
> > - qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
> > - qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
> > - qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
> > - qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
> > - qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4", cxl_2pxb_4rp_4t3d);
> > + qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
> > + qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
> > + qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
> > + qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
> > + qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
> > + qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4",
> > + cxl_2pxb_4rp_4t3d);
> > #endif
> > + } else if (strcmp(arch, "aarch64") == 0) {
> > +#ifdef CONFIG_POSIX
> > + qtest_add_func("/pci/cxl/virt/pxb_x2_root_port_x4_type3_x4",
> > + cxl_virt_2pxb_4rp_4t3d);
> > +#endif
> > + }
> > +
> > return g_test_run();
> > }
> > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> > index 7daf619845..361000267a 100644
> > --- a/tests/qtest/meson.build
> > +++ b/tests/qtest/meson.build
> > @@ -258,6 +258,7 @@ qtests_aarch64 = \
> > (config_all_accel.has_key('CONFIG_TCG') and \
> > config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
> > (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
> > + qtests_cxl + \
> > ['arm-cpu-features',
> > 'numa-test',
> > 'boot-serial-test',
> > --
> > 2.43.0
> >
>
> ~/projects/qemu/build$ meson test qtest-aarch64/cxl-test
> ninja: Entering directory `/home/realm/projects/qemu/build'
> [1/8] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
> 1/1 qemu:qtest+qtest-aarch64 / qtest-aarch64/cxl-test OK 0.17s 1 subtests passed
>
> Ok: 1
> Expected Fail: 0
> Fail: 0
> Unexpected Pass: 0
> Skipped: 0
> Timeout: 0
>
> Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com <mailto:itaru.kitayama@fujitsu.com>>
>
> Jonathan, would you push your branch this series applied? I manually applied your series no issues though.
I'm reluctant to push a 'normal' staging CXL tree whilst we have the TCG
issue outstanding (which is in mainline).
I can probably push one with a name that makes it clear we know it will
crash under some circumstances though. I'll aim to get that done later this week.
After talking to Richard Henderson I'm going to spin some images etc to
make it easier for him to replicate that TCG issue.
Thanks for reviews.
Jonathan
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl
2025-05-16 2:30 ` [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Itaru Kitayama
2025-05-16 6:34 ` Itaru Kitayama
@ 2025-05-20 17:31 ` Jonathan Cameron
1 sibling, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-20 17:31 UTC (permalink / raw)
To: Itaru Kitayama
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
On Fri, 16 May 2025 11:30:49 +0900
Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
> Hi Jonathan,
>
> > On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> >
> > V13:
> > - Make CXL fixed memory windows sysbus devices.
> > IIRC this was requested by Peter in one of the reviews a long time back
> > but at the time the motivation was less strong than it becomes with some
> > WiP patches for hotness monitoring and high performance direct connect
> > where we need a machine type independent way to iterate all the CXL
> > fixed memory windows. This is a convenient place to do it so drag that
> > work forward into this series.
> >
> > This allows us to drop separate list and necessary machine specific
> > access code in favour of
> > object_child_foreach_recursive(object_get_root(),...)
> > One snag is that the ordering of multiple fixed memory windows in that
> > walk depends on the underlying g_hash_table iterations rather than the
> > order of creation. In the memory map layout and ACPI table creation we
> > need both stable and predictable ordering. Resolve this in a similar
> > fashion to object_class_get_list_sorted() be throwing them in a GSList
> > and sorting that. Only use this when a sorted list is needed.
> >
> > Dropped RFC as now I'm happy with this code and would like to get it
> > upstream! Particularly as it broken even today due to enscripten
> > related changes that stop us using g_slist_sort(). Easy fix though.
> >
> > Note that we have an issue for CXL emulation in general and TCG which
> > is being discussed in:
> > https://lore.kernel.org/all/20250425183524.00000b28@huawei.com/
> > (also affects some other platforms)
> >
> > Until that is resolved, either rebase this back on 10.0 or just
> > don't let code run out of it (don't use KMEM to expose it as normal
> > memory, use DAX instead).
> >
> > Previous cover letter.
> >
> > Back in 2022, this series stalled on the absence of a solution to device
> > tree support for PCI Expander Bridges (PXB) and we ended up only having
> > x86 support upstream. I've been carrying the arm64 support out of tree
> > since then, with occasional nasty surprises (e.g. UNIMP + DT issue seen
> > a few weeks ago) and a fair number of fiddly rebases.
> > gitlab.com/jic23/qemu cxl-<latest date>
> >
> > A recent discussion with Peter Maydell indicated that there are various
> > other ACPI only features now, so in general he might be more relaxed
> > about DT support being necessary. The upcoming vSMMUv3 support would
> > run into this problem as well.
> >
> > I presented the background to the PXB issue at Linaro connect 2022. In
> > short the issue is that PXBs steal MMIO space from the main PCI root
> > bridge. The challenge is knowing how much to steal.
> >
> > On ACPI platforms, we can rely on EDK2 to perform an enumeration and
> > configuration of the PCI topology and QEMU can update the ACPI tables
> > after EDK2 has done this when it can simply read the space used by the
> > root ports. On device tree, there is no entity to figure out that
> > enumeration so we don't know how to size the stolen region.
> >
> > Three approaches were discussed:
> > 1) Enumerating in QEMU. Horribly complex and the last thing we want is a
> > 3rd enumeration implementation that ends up out of sync with EDK2 and
> > the kernel (there are frequent issues because of how those existing
> > implementations differ.
> > 2) Figure out how to enumerate in kernel. I never put a huge amount of work
> > into this, but it seemed likely to involve a nasty dance with similar
> > very specific code to that EDK2 is carrying and would very challenging
> > to upstream (given the lack of clarity on real use cases for PXBs and
> > DT).
> > 3) Hack it based on the control we have which is bus numbers.
> > No one liked this but it worked :)
> >
> > The other little wrinkle would be the need to define full bindings for CXL
> > on DT + implement a fairly complex kernel stack as equivalent in ACPI
> > involves a static table, CEDT, new runtime queries via _DSM and a description
> > of various components. Doable, but so far there is no interest on physical
> > platforms. Worth noting that for now, the QEMU CXL emulation is all about
> > testing and developing the OS stack, not about virtualization (performance
> > is terrible except in some very contrived situations!)
> >
> > Back to posting as an RFC because there was some discussion of approach to
> > modelling the devices that may need a bit of redesign.
> > The discussion kind of died out on the back of DT issue and I doubt anyone
> > can remember the details.
> >
> > https://lore.kernel.org/qemu-devel/20220616141950.23374-1-Jonathan.Cameron@huawei.com/
> >
> > There is only a very simple test in here, because my intent is not to
> > duplicate what we have on x86, but just to do a smoke test that everything
> > is hooked up. In general we need much more comprehensive end to end CXL
> > tests but that requires a reaonsably stable guest software stack. A few
> > people have expressed interest in working on that, but we aren't there yet.
> >
> > Note that this series has a very different use case to that in the proposed
> > SBSA-ref support:
> > https://lore.kernel.org/qemu-devel/20250117034343.26356-1-wangyuquan1236@phytium.com.cn/
> >
> > SBSA-ref is a good choice if you want a relatively simple mostly fixed
> > configuration. That works well with the limited host system
> > discoverability etc as EDK2 can be build against a known configuration.
> >
> > My interest with this support in arm/virt is support host software stack
> > development (we have a wide range of contributors, most of whom are working
> > on emulation + the kernel support). I care about the weird corners. As such
> > I need to be able to bring up variable numbers of host bridges, multiple CXL
> > Fixed Memory Windows with varying characteristics (interleave etc), complex
> > NUMA topologies with wierd performance characteristics etc. We can do that
> > on x86 upstream today, or my gitlab tree. Note that we need arm support
> > for some arch specific features in the near future (cache flushing).
> > Doing kernel development with this need for flexibility on SBSA-ref is not
> > currently practical. SBSA-ref CXL support is an excellent thing, just
> > not much use to me for this work.
> >
> > Jonathan Cameron (5):
> > hw/cxl-host: Add an index field to CXLFixedMemoryWindow
> > hw/cxl: Make the CXL fixed memory windows devices.
> > hw/cxl-host: Allow split of establishing memory address and mmio
> > setup.
> > hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances
> > pxb-cxl
> > qtest/cxl: Add aarch64 virt test for CXL
> >
> > include/hw/arm/virt.h | 4 +
> > include/hw/cxl/cxl.h | 4 +
> > include/hw/cxl/cxl_host.h | 6 +-
> > hw/acpi/cxl.c | 83 +++++++++------
> > hw/arm/virt-acpi-build.c | 34 ++++++
> > hw/arm/virt.c | 29 +++++
> > hw/cxl/cxl-host-stubs.c | 8 +-
> > hw/cxl/cxl-host.c | 218 ++++++++++++++++++++++++++++++++------
> > hw/i386/pc.c | 51 ++++-----
> > tests/qtest/cxl-test.c | 59 ++++++++---
> > tests/qtest/meson.build | 1 +
> > 11 files changed, 389 insertions(+), 108 deletions(-)
> >
> > --
> > 2.43.0
> >
>
> With your series applied on top of upstream QEMU, the -drive option does not work well with the sane CXL
> setup (I use run_qemu.sh maintained by Marc et al. at Intel) see below:
>
> /home/realm/projects/qemu/build/qemu-system-aarch64 -machine virt,accel=tcg,cxl=on,highmem=on,compact-highmem=on,highmem-ecam=on,highmem-mmio=on -m 2048M,slots=0,maxmem=6144M -smp 2,sockets=1,cores=2,threads=1 -display none -nographic -drive if=pflash,format=raw,unit=0,file=AAVMF_CODE.fd,readonly=on -drive if=pflash,format=raw,unit=1,file=AAVMF_VARS.fd -drive file=root.img,format=raw,media=disk -kernel mkosi.extra/boot/vmlinuz-6.15.0-rc4-00040-g128ad8fa385b -initrd mkosi.extra/boot/initramfs-6.15.0-rc4-00040-g128ad8fa385b.img -append selinux=0 audit=0 console=tty0 console=ttyS0 root=PARTUUID=14d6bae9-c917-435d-89ea-99af1fa4439a ignore_loglevel rw initcall_debug log_buf_len=20M memory_hotplug.memmap_on_memory=force cxl_acpi.dyndbg=+fplm cxl_pci.dyndbg=+fplm cxl_core.dyndbg=+fplm cxl_mem.dyndbg=+fplm cxl_pmem.dyndbg=+fplm cxl_port.dyndbg=+fplm cxl_region.dyndbg=+fplm cxl_test.dyndbg=+fplm cxl_mock.dyndbg=+fplm cxl_mock_mem.dyndbg=+fplm systemd.set_credential=agetty.autologin:root systemd.set_credential=login.noauth:yes -device e1000,netdev=net0,mac=52:54:00:12:34:56 -netdev user,id=net0,hostfwd=tcp::10022-:22 -cpu max -object memory-backend-file,id=cxl-mem0,share=on,mem-path=cxltest0.raw,size=256M -object memory-backend-file,id=cxl-mem1,share=on,mem-path=cxltest1.raw,size=256M -object memory-backend-file,id=cxl-mem2,share=on,mem-path=cxltest2.raw,size=256M -object memory-backend-file,id=cxl-mem3,share=on,mem-path=cxltest3.raw,size=256M -object memory-backend-file,id=cxl-lsa0,share=on,mem-path=lsa0.raw,size=128K -object memory-backend-file,id=cxl-lsa1,share=on,mem-path=lsa1.raw,size=128K -object memory-backend-file,id=cxl-lsa2,share=on,mem-path=lsa2.raw,size=128K -object memory-backend-file,id=cxl-lsa3,share=on,mem-path=lsa3.raw,size=128K -device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=53 -device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=191 -device cxl-rp,id=hb0rp0,bus=cxl.0,chassis=0,slot=0,port=0 -device cxl-rp,id=hb0rp1,bus=cxl.0,chassis=0,slot=1,port=1 -device cxl-rp,id=hb1rp0,bus=cxl.1,chassis=0,slot=2,port=0 -device cxl-rp,id=hb1rp1,bus=cxl.1,chassis=0,slot=3,port=1 -device cxl-upstream,port=4,bus=hb0rp0,id=cxl-up0,multifunction=on,addr=0.0,sn=12345678 -device cxl-switch-mailbox-cci,bus=hb0rp0,addr=0.1,target=cxl-up0 -device cxl-upstream,port=4,bus=hb1rp0,id=cxl-up1,multifunction=on,addr=0.0,sn=12341234 -device cxl-switch-mailbox-cci,bus=hb1rp0,addr=0.1,target=cxl-up1 -device cxl-downstream,port=0,bus=cxl-up0,id=swport0,chassis=0,slot=4 -device cxl-downstream,port=1,bus=cxl-up0,id=swport1,chassis=0,slot=5 -device cxl-downstream,port=2,bus=cxl-up0,id=swport2,chassis=0,slot=6 -device cxl-downstream,port=3,bus=cxl-up0,id=swport3,chassis=0,slot=7 -device cxl-downstream,port=0,bus=cxl-up1,id=swport4,chassis=0,slot=8 -device cxl-downstream,port=1,bus=cxl-up1,id=swport5,chassis=0,slot=9 -device cxl-downstream,port=2,bus=cxl-up1,id=swport6,chassis=0,slot=10 -device cxl-downstream,port=3,bus=cxl-up1,id=swport7,chassis=0,slot=11 -device cxl-type3,bus=swport0,persistent-memdev=cxl-mem0,id=cxl-pmem0,lsa=cxl-lsa0 -device cxl-type3,bus=swport2,persistent-memdev=cxl-mem1,id=cxl-pmem1,lsa=cxl-lsa1 -device cxl-type3,bus=swport4,volatile-memdev=cxl-mem2,id=cxl-vmem2,lsa=cxl-lsa2 -device cxl-type3,bus=swport6,volatile-memdev=cxl-mem3,id=cxl-vmem3,lsa=cxl-lsa3 -M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=8k,cxl-fmw.1.targets.0=cxl.0,cxl-fmw.1.targets.1=cxl.1,cxl-fmw.1.size=4G,cxl-fmw.1.interleave-granularity=8k -snapshot -object memory-backend-ram,id=mem0,size=2048M -numa node,nodeid=0,memdev=mem0, -numa cpu,node-id=0,socket-id=0 -numa dist,src=0,dst=0,val=10
> qemu-system-aarch64: -drive file=root.img,format=raw,media=disk: PCI: Only PCI/PCIe bridges can be plugged into pxb-cxl
>
> Plain upstream QEMU aarch64 target vert machine can handle the -drive option without an issue _without_ those cxl setup options added. I think the error was seen with your previous cxl-2025-03-20 branch.
>
I think I understand what is going on here.
This is a challenge to fix at qemu side because it is relying
on a bunch of old behavior. For a while now legacy -drive parameters have been
split into parts that go in -drive and a separate -device entry
that covers the hardware - even with that though we'd need
to specify a bus as on arm64 the default for -drive is virtio-blk
which ends up I connected on the pcie bus.
Note this is just as broken if you use any pxb-pcie instances as
those have similar fixed assumption that you can only hang root ports
off them.
I think best solution is probably to stop use the old style
-drive file=root.img,format=raw,media=disk
and switch to fully expanded virtio solution along the lines of
-drive if=none,file=root.img,format=raw,media=disk,id=hd \
-device virtio-blk,bus=pcie.0,drive=hd
which should be fine on x86 and arm64 + any other architectures we
support in the future and will always bring the disk up as an RCiEP
on the main pcie root complex.
Jonathan
> Thanks,
> Itaru.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL
2025-05-19 12:54 ` Jonathan Cameron
@ 2025-05-20 20:38 ` Itaru Kitayama
2025-05-21 7:38 ` Itaru Kitayama
1 sibling, 0 replies; 24+ messages in thread
From: Itaru Kitayama @ 2025-05-20 20:38 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
Jonathan,
> On May 19, 2025, at 21:54, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>
> On Thu, 15 May 2025 18:04:18 +0900
> Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
>
>>> On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>>>
>>> Add a single complex case for aarch64 virt machine.
>>> Given existing much more comprehensive tests for x86 cover the
>>> common functionality, a single test should be enough to verify
>>> that the aarch64 part continue to work.
>>>
>>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>> ---
>>> tests/qtest/cxl-test.c | 59 ++++++++++++++++++++++++++++++++---------
>>> tests/qtest/meson.build | 1 +
>>> 2 files changed, 47 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
>>> index a600331843..c7189d6222 100644
>>> --- a/tests/qtest/cxl-test.c
>>> +++ b/tests/qtest/cxl-test.c
>>> @@ -19,6 +19,12 @@
>>> "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
>>> "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
>>>
>>> +#define QEMU_VIRT_2PXB_CMD \
>>> + "-machine virt,cxl=on -cpu max " \
>>> + "-device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52 " \
>>> + "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
>>> + "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
>>> +
>>> #define QEMU_RP \
>>> "-device cxl-rp,id=rp0,bus=cxl.0,chassis=0,slot=0 "
>>>
>>> @@ -197,25 +203,52 @@ static void cxl_2pxb_4rp_4t3d(void)
>>> qtest_end();
>>> rmdir(tmpfs);
>>> }
>>> +
>>> +static void cxl_virt_2pxb_4rp_4t3d(void)
>>> +{
>>> + g_autoptr(GString) cmdline = g_string_new(NULL);
>>> + char template[] = "/tmp/cxl-test-XXXXXX";
>>> + const char *tmpfs;
>>> +
>>> + tmpfs = mkdtemp(template);
>>> +
>>> + g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
>>> + tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
>>> + tmpfs, tmpfs);
>>> +
>>> + qtest_start(cmdline->str);
>>> + qtest_end();
>>> + rmdir(tmpfs);
>>> +}
>>> #endif /* CONFIG_POSIX */
>>>
>>> int main(int argc, char **argv)
>>> {
>>> - g_test_init(&argc, &argv, NULL);
>>> + const char *arch = qtest_get_arch();
>>>
>>> - qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
>>> - qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
>>> - qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
>>> - qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
>>> - qtest_add_func("/pci/cxl/rp", cxl_root_port);
>>> - qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
>>> + g_test_init(&argc, &argv, NULL);
>>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>> + qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
>>> + qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
>>> + qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
>>> + qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
>>> + qtest_add_func("/pci/cxl/rp", cxl_root_port);
>>> + qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
>>> #ifdef CONFIG_POSIX
>>> - qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
>>> - qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
>>> - qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
>>> - qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
>>> - qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
>>> - qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4", cxl_2pxb_4rp_4t3d);
>>> + qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
>>> + qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
>>> + qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
>>> + qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
>>> + qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
>>> + qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4",
>>> + cxl_2pxb_4rp_4t3d);
>>> #endif
>>> + } else if (strcmp(arch, "aarch64") == 0) {
>>> +#ifdef CONFIG_POSIX
>>> + qtest_add_func("/pci/cxl/virt/pxb_x2_root_port_x4_type3_x4",
>>> + cxl_virt_2pxb_4rp_4t3d);
>>> +#endif
>>> + }
>>> +
>>> return g_test_run();
>>> }
>>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>>> index 7daf619845..361000267a 100644
>>> --- a/tests/qtest/meson.build
>>> +++ b/tests/qtest/meson.build
>>> @@ -258,6 +258,7 @@ qtests_aarch64 = \
>>> (config_all_accel.has_key('CONFIG_TCG') and \
>>> config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
>>> (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
>>> + qtests_cxl + \
>>> ['arm-cpu-features',
>>> 'numa-test',
>>> 'boot-serial-test',
>>> --
>>> 2.43.0
>>>
>>
>> ~/projects/qemu/build$ meson test qtest-aarch64/cxl-test
>> ninja: Entering directory `/home/realm/projects/qemu/build'
>> [1/8] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
>> 1/1 qemu:qtest+qtest-aarch64 / qtest-aarch64/cxl-test OK 0.17s 1 subtests passed
>>
>> Ok: 1
>> Expected Fail: 0
>> Fail: 0
>> Unexpected Pass: 0
>> Skipped: 0
>> Timeout: 0
>>
>> Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com <mailto:itaru.kitayama@fujitsu.com>>
>>
>> Jonathan, would you push your branch this series applied? I manually applied your series no issues though.
>
> I'm reluctant to push a 'normal' staging CXL tree whilst we have the TCG
> issue outstanding (which is in mainline).
> I can probably push one with a name that makes it clear we know it will
> crash under some circumstances though. I'll aim to get that done later this week.
Okay, and understood.
>
> After talking to Richard Henderson I'm going to spin some images etc to
> make it easier for him to replicate that TCG issue.
>
> Thanks for reviews.
The comments are from Zhijian.
Itaru.
>
> Jonathan
>
>>
>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL
2025-05-19 12:54 ` Jonathan Cameron
2025-05-20 20:38 ` Itaru Kitayama
@ 2025-05-21 7:38 ` Itaru Kitayama
2025-05-21 17:52 ` Jonathan Cameron
1 sibling, 1 reply; 24+ messages in thread
From: Itaru Kitayama @ 2025-05-21 7:38 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
> On May 19, 2025, at 21:54, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>
> On Thu, 15 May 2025 18:04:18 +0900
> Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
>
>>> On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>>>
>>> Add a single complex case for aarch64 virt machine.
>>> Given existing much more comprehensive tests for x86 cover the
>>> common functionality, a single test should be enough to verify
>>> that the aarch64 part continue to work.
>>>
>>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>> ---
>>> tests/qtest/cxl-test.c | 59 ++++++++++++++++++++++++++++++++---------
>>> tests/qtest/meson.build | 1 +
>>> 2 files changed, 47 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
>>> index a600331843..c7189d6222 100644
>>> --- a/tests/qtest/cxl-test.c
>>> +++ b/tests/qtest/cxl-test.c
>>> @@ -19,6 +19,12 @@
>>> "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
>>> "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
>>>
>>> +#define QEMU_VIRT_2PXB_CMD \
>>> + "-machine virt,cxl=on -cpu max " \
>>> + "-device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52 " \
>>> + "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
>>> + "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
>>> +
>>> #define QEMU_RP \
>>> "-device cxl-rp,id=rp0,bus=cxl.0,chassis=0,slot=0 "
>>>
>>> @@ -197,25 +203,52 @@ static void cxl_2pxb_4rp_4t3d(void)
>>> qtest_end();
>>> rmdir(tmpfs);
>>> }
>>> +
>>> +static void cxl_virt_2pxb_4rp_4t3d(void)
>>> +{
>>> + g_autoptr(GString) cmdline = g_string_new(NULL);
>>> + char template[] = "/tmp/cxl-test-XXXXXX";
>>> + const char *tmpfs;
>>> +
>>> + tmpfs = mkdtemp(template);
>>> +
>>> + g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
>>> + tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
>>> + tmpfs, tmpfs);
>>> +
>>> + qtest_start(cmdline->str);
>>> + qtest_end();
>>> + rmdir(tmpfs);
>>> +}
>>> #endif /* CONFIG_POSIX */
>>>
>>> int main(int argc, char **argv)
>>> {
>>> - g_test_init(&argc, &argv, NULL);
>>> + const char *arch = qtest_get_arch();
>>>
>>> - qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
>>> - qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
>>> - qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
>>> - qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
>>> - qtest_add_func("/pci/cxl/rp", cxl_root_port);
>>> - qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
>>> + g_test_init(&argc, &argv, NULL);
>>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>> + qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
>>> + qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
>>> + qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
>>> + qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
>>> + qtest_add_func("/pci/cxl/rp", cxl_root_port);
>>> + qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
>>> #ifdef CONFIG_POSIX
>>> - qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
>>> - qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
>>> - qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
>>> - qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
>>> - qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
>>> - qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4", cxl_2pxb_4rp_4t3d);
>>> + qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
>>> + qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
>>> + qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
>>> + qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
>>> + qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
>>> + qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4",
>>> + cxl_2pxb_4rp_4t3d);
>>> #endif
>>> + } else if (strcmp(arch, "aarch64") == 0) {
>>> +#ifdef CONFIG_POSIX
>>> + qtest_add_func("/pci/cxl/virt/pxb_x2_root_port_x4_type3_x4",
>>> + cxl_virt_2pxb_4rp_4t3d);
>>> +#endif
>>> + }
>>> +
>>> return g_test_run();
>>> }
>>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>>> index 7daf619845..361000267a 100644
>>> --- a/tests/qtest/meson.build
>>> +++ b/tests/qtest/meson.build
>>> @@ -258,6 +258,7 @@ qtests_aarch64 = \
>>> (config_all_accel.has_key('CONFIG_TCG') and \
>>> config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
>>> (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
>>> + qtests_cxl + \
>>> ['arm-cpu-features',
>>> 'numa-test',
>>> 'boot-serial-test',
>>> --
>>> 2.43.0
>>>
>>
>> ~/projects/qemu/build$ meson test qtest-aarch64/cxl-test
>> ninja: Entering directory `/home/realm/projects/qemu/build'
>> [1/8] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
>> 1/1 qemu:qtest+qtest-aarch64 / qtest-aarch64/cxl-test OK 0.17s 1 subtests passed
>>
>> Ok: 1
>> Expected Fail: 0
>> Fail: 0
>> Unexpected Pass: 0
>> Skipped: 0
>> Timeout: 0
>>
>> Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com <mailto:itaru.kitayama@fujitsu.com>>
>>
>> Jonathan, would you push your branch this series applied? I manually applied your series no issues though.
>
> I'm reluctant to push a 'normal' staging CXL tree whilst we have the TCG
> issue outstanding (which is in mainline).
> I can probably push one with a name that makes it clear we know it will
> crash under some circumstances though. I'll aim to get that done later this week.
>
> After talking to Richard Henderson I'm going to spin some images etc to
> make it easier for him to replicate that TCG issue.
While QEMU (the kernel is built off of cxl branch) boots fine and lspci shows CXL devices as shown:
root@localhost:~# lspci -mm
00:00.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Host bridge" -p00 "Red Hat, Inc." "Device 1100"
00:01.0 "SCSI storage controller" "Red Hat, Inc." "Virtio block device" -p00 "Red Hat, Inc." "Device 0002"
00:02.0 "Ethernet controller" "Intel Corporation" "82540EM Gigabit Ethernet Controller" -r03 -p00 "Red Hat, Inc." "QEMU Virtual Machine"
00:03.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Expander bridge" -p00 "Red Hat, Inc." "Device 1100"
00:04.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Expander bridge" -p00 "Red Hat, Inc." "Device 1100"
35:00.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
35:01.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
36:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a128" -p00 "" ""
36:00.1 "Serial bus controller [0c0b]" "Huawei Technologies Co., Ltd." "Device a123" -p00 "Red Hat, Inc." "Device 1100"
37:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
37:01.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
37:02.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
37:03.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
38:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
3a:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
bf:00.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
bf:01.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
c0:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a128" -p00 "" ""
c0:00.1 "Serial bus controller [0c0b]" "Huawei Technologies Co., Ltd." "Device a123" -p00 "Red Hat, Inc." "Device 1100"
c1:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
c1:01.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
c1:02.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
c1:03.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
c2:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
c4:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100”
but the cxl-list command takes 5-10 minutes to return the information. I’ll test with your qtest minimalist setup to see if the system is loaded. Am I seeing the TCG issue you mentioned?
root@localhost:~# cxl list -M
[
{
"memdev":"mem10",
"ram_size":2147483648,
"ram_qos_class":42,
"serial":11,
"numa_node":0,
"host":"cxl_rcd.10",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem5",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":6,
"numa_node":1,
"host":"cxl_mem.5",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem1",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":2,
"numa_node":1,
"host":"cxl_mem.1",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem6",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":8,
"numa_node":1,
"host":"cxl_mem.7",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem3",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":4,
"numa_node":1,
"host":"cxl_mem.3",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem4",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":5,
"numa_node":0,
"host":"cxl_mem.4",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem0",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":1,
"numa_node":0,
"host":"cxl_mem.0",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem2",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":3,
"numa_node":0,
"host":"cxl_mem.2",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem7",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":7,
"numa_node":0,
"host":"cxl_mem.6",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem8",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":9,
"numa_node":0,
"host":"cxl_mem.8",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem9",
"pmem_size":1073741824,
"pmem_qos_class":42,
"ram_size":1073741824,
"ram_qos_class":42,
"serial":10,
"numa_node":1,
"host":"cxl_mem.9",
"firmware_version":"mock fw v1 "
},
{
"memdev":"mem12",
"ram_size":268435456,
"serial":0,
"host":"0000:c4:00.0",
"firmware_version":"BWFW VERSION 00"
},
{
"memdev":"mem11",
"ram_size":268435456,
"serial":0,
"host":"0000:c2:00.0",
"firmware_version":"BWFW VERSION 00"
},
{
"memdev":"mem14",
"pmem_size":268435456,
"serial":0,
"host":"0000:3a:00.0",
"firmware_version":"BWFW VERSION 00"
},
{
"memdev":"mem13",
"pmem_size":268435456,
"serial":0,
"host":"0000:38:00.0",
"firmware_version":"BWFW VERSION 00"
}
]
Getting this shouldn’t take minutes, even with the emulator I think.
Itaru.
>
> Thanks for reviews.
>
> Jonathan
>
>>
>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow
2025-05-13 11:14 ` [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow Jonathan Cameron
2025-05-16 5:44 ` Zhijian Li (Fujitsu)
@ 2025-05-21 16:59 ` Fan Ni
2025-08-08 8:29 ` wangyuquan
2025-08-08 8:57 ` Yuquan Wang
3 siblings, 0 replies; 24+ messages in thread
From: Fan Ni @ 2025-05-21 16:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, Peter Maydell, mst, linux-cxl, linuxarm, qemu-arm,
Yuquan Wang, Itaru Kitayama, Philippe Mathieu-Daudé
On Tue, May 13, 2025 at 12:14:51PM +0100, Jonathan Cameron wrote:
> To enable these to be found in a fixed order, that order needs
> to be known. This will later be used to sort a list of these
> structures that address map and ACPI table entries are predictable.
>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
Reviewed-by: Fan Ni <fan.ni@samsung.com>
> include/hw/cxl/cxl.h | 1 +
> hw/cxl/cxl-host.c | 9 ++++++---
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> index 75e47b6864..b2bcce7ed6 100644
> --- a/include/hw/cxl/cxl.h
> +++ b/include/hw/cxl/cxl.h
> @@ -27,6 +27,7 @@
> typedef struct PXBCXLDev PXBCXLDev;
>
> typedef struct CXLFixedWindow {
> + int index;
> uint64_t size;
> char **targets;
> PXBCXLDev *target_hbs[16];
> diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> index e010163174..b7aa429ddf 100644
> --- a/hw/cxl/cxl-host.c
> +++ b/hw/cxl/cxl-host.c
> @@ -24,13 +24,15 @@
>
> static void cxl_fixed_memory_window_config(CXLState *cxl_state,
> CXLFixedMemoryWindowOptions *object,
> - Error **errp)
> + int index, Error **errp)
> {
> ERRP_GUARD();
> g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
> strList *target;
> int i;
>
> + fw->index = index;
> +
> for (target = object->targets; target; target = target->next) {
> fw->num_targets++;
> }
> @@ -325,14 +327,15 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
> CXLState *state = opaque;
> CXLFixedMemoryWindowOptionsList *cfmw_list = NULL;
> CXLFixedMemoryWindowOptionsList *it;
> + int index;
>
> visit_type_CXLFixedMemoryWindowOptionsList(v, name, &cfmw_list, errp);
> if (!cfmw_list) {
> return;
> }
>
> - for (it = cfmw_list; it; it = it->next) {
> - cxl_fixed_memory_window_config(state, it->value, errp);
> + for (it = cfmw_list, index = 0; it; it = it->next, index++) {
> + cxl_fixed_memory_window_config(state, it->value, index, errp);
> }
> state->cfmw_list = cfmw_list;
> }
> --
> 2.43.0
>
--
Fan Ni (From gmail)
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL
2025-05-21 7:38 ` Itaru Kitayama
@ 2025-05-21 17:52 ` Jonathan Cameron
2025-05-21 22:07 ` Itaru Kitayama
0 siblings, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-21 17:52 UTC (permalink / raw)
To: Itaru Kitayama
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
On Wed, 21 May 2025 16:38:10 +0900
Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
> > On May 19, 2025, at 21:54, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Thu, 15 May 2025 18:04:18 +0900
> > Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
> >
> >>> On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> >>>
> >>> Add a single complex case for aarch64 virt machine.
> >>> Given existing much more comprehensive tests for x86 cover the
> >>> common functionality, a single test should be enough to verify
> >>> that the aarch64 part continue to work.
> >>>
> >>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >>> ---
> >>> tests/qtest/cxl-test.c | 59 ++++++++++++++++++++++++++++++++---------
> >>> tests/qtest/meson.build | 1 +
> >>> 2 files changed, 47 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
> >>> index a600331843..c7189d6222 100644
> >>> --- a/tests/qtest/cxl-test.c
> >>> +++ b/tests/qtest/cxl-test.c
> >>> @@ -19,6 +19,12 @@
> >>> "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
> >>> "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
> >>>
> >>> +#define QEMU_VIRT_2PXB_CMD \
> >>> + "-machine virt,cxl=on -cpu max " \
> >>> + "-device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52 " \
> >>> + "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
> >>> + "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
> >>> +
> >>> #define QEMU_RP \
> >>> "-device cxl-rp,id=rp0,bus=cxl.0,chassis=0,slot=0 "
> >>>
> >>> @@ -197,25 +203,52 @@ static void cxl_2pxb_4rp_4t3d(void)
> >>> qtest_end();
> >>> rmdir(tmpfs);
> >>> }
> >>> +
> >>> +static void cxl_virt_2pxb_4rp_4t3d(void)
> >>> +{
> >>> + g_autoptr(GString) cmdline = g_string_new(NULL);
> >>> + char template[] = "/tmp/cxl-test-XXXXXX";
> >>> + const char *tmpfs;
> >>> +
> >>> + tmpfs = mkdtemp(template);
> >>> +
> >>> + g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
> >>> + tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
> >>> + tmpfs, tmpfs);
> >>> +
> >>> + qtest_start(cmdline->str);
> >>> + qtest_end();
> >>> + rmdir(tmpfs);
> >>> +}
> >>> #endif /* CONFIG_POSIX */
> >>>
> >>> int main(int argc, char **argv)
> >>> {
> >>> - g_test_init(&argc, &argv, NULL);
> >>> + const char *arch = qtest_get_arch();
> >>>
> >>> - qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
> >>> - qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
> >>> - qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
> >>> - qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
> >>> - qtest_add_func("/pci/cxl/rp", cxl_root_port);
> >>> - qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
> >>> + g_test_init(&argc, &argv, NULL);
> >>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> >>> + qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
> >>> + qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
> >>> + qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
> >>> + qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
> >>> + qtest_add_func("/pci/cxl/rp", cxl_root_port);
> >>> + qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
> >>> #ifdef CONFIG_POSIX
> >>> - qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
> >>> - qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
> >>> - qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
> >>> - qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
> >>> - qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
> >>> - qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4", cxl_2pxb_4rp_4t3d);
> >>> + qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
> >>> + qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
> >>> + qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
> >>> + qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
> >>> + qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
> >>> + qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4",
> >>> + cxl_2pxb_4rp_4t3d);
> >>> #endif
> >>> + } else if (strcmp(arch, "aarch64") == 0) {
> >>> +#ifdef CONFIG_POSIX
> >>> + qtest_add_func("/pci/cxl/virt/pxb_x2_root_port_x4_type3_x4",
> >>> + cxl_virt_2pxb_4rp_4t3d);
> >>> +#endif
> >>> + }
> >>> +
> >>> return g_test_run();
> >>> }
> >>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> >>> index 7daf619845..361000267a 100644
> >>> --- a/tests/qtest/meson.build
> >>> +++ b/tests/qtest/meson.build
> >>> @@ -258,6 +258,7 @@ qtests_aarch64 = \
> >>> (config_all_accel.has_key('CONFIG_TCG') and \
> >>> config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
> >>> (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
> >>> + qtests_cxl + \
> >>> ['arm-cpu-features',
> >>> 'numa-test',
> >>> 'boot-serial-test',
> >>> --
> >>> 2.43.0
> >>>
> >>
> >> ~/projects/qemu/build$ meson test qtest-aarch64/cxl-test
> >> ninja: Entering directory `/home/realm/projects/qemu/build'
> >> [1/8] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
> >> 1/1 qemu:qtest+qtest-aarch64 / qtest-aarch64/cxl-test OK 0.17s 1 subtests passed
> >>
> >> Ok: 1
> >> Expected Fail: 0
> >> Fail: 0
> >> Unexpected Pass: 0
> >> Skipped: 0
> >> Timeout: 0
> >>
> >> Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com <mailto:itaru.kitayama@fujitsu.com>>
> >>
> >> Jonathan, would you push your branch this series applied? I manually applied your series no issues though.
> >
> > I'm reluctant to push a 'normal' staging CXL tree whilst we have the TCG
> > issue outstanding (which is in mainline).
> > I can probably push one with a name that makes it clear we know it will
> > crash under some circumstances though. I'll aim to get that done later this week.
> >
> > After talking to Richard Henderson I'm going to spin some images etc to
> > make it easier for him to replicate that TCG issue.
>
> While QEMU (the kernel is built off of cxl branch) boots fine and lspci shows CXL devices as shown:
>
> root@localhost:~# lspci -mm
> 00:00.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Host bridge" -p00 "Red Hat, Inc." "Device 1100"
> 00:01.0 "SCSI storage controller" "Red Hat, Inc." "Virtio block device" -p00 "Red Hat, Inc." "Device 0002"
> 00:02.0 "Ethernet controller" "Intel Corporation" "82540EM Gigabit Ethernet Controller" -r03 -p00 "Red Hat, Inc." "QEMU Virtual Machine"
> 00:03.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Expander bridge" -p00 "Red Hat, Inc." "Device 1100"
> 00:04.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Expander bridge" -p00 "Red Hat, Inc." "Device 1100"
> 35:00.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
> 35:01.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
> 36:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a128" -p00 "" ""
> 36:00.1 "Serial bus controller [0c0b]" "Huawei Technologies Co., Ltd." "Device a123" -p00 "Red Hat, Inc." "Device 1100"
> 37:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
> 37:01.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
> 37:02.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
> 37:03.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
> 38:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
> 3a:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
> bf:00.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
> bf:01.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
> c0:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a128" -p00 "" ""
> c0:00.1 "Serial bus controller [0c0b]" "Huawei Technologies Co., Ltd." "Device a123" -p00 "Red Hat, Inc." "Device 1100"
> c1:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
> c1:01.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
> c1:02.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
> c1:03.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
> c2:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
> c4:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100”
>
> but the cxl-list command takes 5-10 minutes to return the information. I’ll test with your qtest minimalist setup to see if the system is loaded. Am I seeing the TCG issue you mentioned?
I don't think so. That should only matter if the memory is hotplugged in
as normal system RAM.
>
> root@localhost:~# cxl list -M
> [
> {
> "memdev":"mem10",
> "ram_size":2147483648,
> "ram_qos_class":42,
> "serial":11,
> "numa_node":0,
> "host":"cxl_rcd.10",
> "firmware_version":"mock fw v1 "
So this seems to be with both some emulated devices from qemu command line
and CXL test at the same time.
I replicated a similar setup and not setting a significant delay
(half a second maybe?) So not sure.
I did notice that there is a bug in qemu though which has surfaced
for some reason in the kernel log (it dates all the way back to
origin CXL support series). In
include/hw/cxl/cxl_pci.h
#define REG_LOC_DEVSEC_LENGTH is 0x24 and it should be 0x1C
That results for me in a kernel log entry about a bar not being
big enough for a huge offset (which is random data coming from
somewhere in text I think).
Seems unlikely to trigger your issue, but you never know!
Jonathan
> },
> {
> "memdev":"mem5",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":6,
> "numa_node":1,
> "host":"cxl_mem.5",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem1",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":2,
> "numa_node":1,
> "host":"cxl_mem.1",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem6",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":8,
> "numa_node":1,
> "host":"cxl_mem.7",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem3",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":4,
> "numa_node":1,
> "host":"cxl_mem.3",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem4",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":5,
> "numa_node":0,
> "host":"cxl_mem.4",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem0",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":1,
> "numa_node":0,
> "host":"cxl_mem.0",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem2",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":3,
> "numa_node":0,
> "host":"cxl_mem.2",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem7",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":7,
> "numa_node":0,
> "host":"cxl_mem.6",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem8",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":9,
> "numa_node":0,
> "host":"cxl_mem.8",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem9",
> "pmem_size":1073741824,
> "pmem_qos_class":42,
> "ram_size":1073741824,
> "ram_qos_class":42,
> "serial":10,
> "numa_node":1,
> "host":"cxl_mem.9",
> "firmware_version":"mock fw v1 "
> },
> {
> "memdev":"mem12",
> "ram_size":268435456,
> "serial":0,
> "host":"0000:c4:00.0",
> "firmware_version":"BWFW VERSION 00"
> },
> {
> "memdev":"mem11",
> "ram_size":268435456,
> "serial":0,
> "host":"0000:c2:00.0",
> "firmware_version":"BWFW VERSION 00"
> },
> {
> "memdev":"mem14",
> "pmem_size":268435456,
> "serial":0,
> "host":"0000:3a:00.0",
> "firmware_version":"BWFW VERSION 00"
> },
> {
> "memdev":"mem13",
> "pmem_size":268435456,
> "serial":0,
> "host":"0000:38:00.0",
> "firmware_version":"BWFW VERSION 00"
> }
> ]
>
> Getting this shouldn’t take minutes, even with the emulator I think.
>
> Itaru.
>
> >
> > Thanks for reviews.
> >
> > Jonathan
> >
> >>
> >>
> >
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices.
2025-05-16 5:44 ` Zhijian Li (Fujitsu)
@ 2025-05-21 17:54 ` Fan Ni
2025-05-27 16:04 ` Jonathan Cameron
1 sibling, 0 replies; 24+ messages in thread
From: Fan Ni @ 2025-05-21 17:54 UTC (permalink / raw)
To: Zhijian Li (Fujitsu)
Cc: Jonathan Cameron, qemu-devel@nongnu.org, Peter Maydell,
mst@redhat.com, linux-cxl@vger.kernel.org, linuxarm@huawei.com,
qemu-arm@nongnu.org, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
On Fri, May 16, 2025 at 05:44:34AM +0000, Zhijian Li (Fujitsu) wrote:
>
>
> On 13/05/2025 19:14, Jonathan Cameron via wrote:
> > Previously these somewhat device like structures were tracked using a list
> > in the CXLState in each machine. This is proving restrictive in a few
> > cases where we need to iterate through these without being aware of the
> > machine type. Just make them sysbus devices.
> Make sense.
>
> Minor comments inline
>
> >
> > Restrict them to not user created as they need to be visible to early
> > stages of machine init given effects on the memory map.
> >
> > This change both simplifies state tracking and enables features needed
> > for performance optimization and hotness tracking by making it possible
> > to retrieve the fixed memory window on actions elsewhere in the topology.
> >
> > In some cases the ordering of the Fixed Memory Windows matters.
> > For those utility functions provide a GSList sorted by the window index.
> > This ensures that we get consistency across:
> > - ordering in the command line
> > - ordering of the host PA ranges
> > - ordering of ACPI CEDT structures describing the CFMWS.
> >
> > Other aspects don't have this constraint. For those direct iteration
> > of the underlying hash structures is fine.
> >
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > I think Peter Maydell suggested this a long time back when
> > the original CXL support series was under review but not 100% sure.
> > ---
> > include/hw/cxl/cxl.h | 3 +
> > include/hw/cxl/cxl_host.h | 4 +-
> > hw/acpi/cxl.c | 83 +++++++++++--------
> > hw/cxl/cxl-host-stubs.c | 6 +-
> > hw/cxl/cxl-host.c | 169 +++++++++++++++++++++++++++++++-------
> > hw/i386/pc.c | 51 ++++++------
> > 6 files changed, 223 insertions(+), 93 deletions(-)
> >
> > diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> > index b2bcce7ed6..a610795c87 100644
> > --- a/include/hw/cxl/cxl.h
> > +++ b/include/hw/cxl/cxl.h
> > @@ -27,6 +27,7 @@
> > typedef struct PXBCXLDev PXBCXLDev;
> >
> > typedef struct CXLFixedWindow {
> > + SysBusDevice parent_obj;
> > int index;
> > uint64_t size;
> > char **targets;
> > @@ -38,6 +39,8 @@ typedef struct CXLFixedWindow {
> > MemoryRegion mr;
> > hwaddr base;
> > } CXLFixedWindow;
> > +#define TYPE_CXL_FMW "cxl-fmw"
> > +OBJECT_DECLARE_SIMPLE_TYPE(CXLFixedWindow, CXL_FMW)
> >
> > typedef struct CXLState {
> > bool is_enabled;
> > diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
> > index c9bc9c7c50..6dce2cde07 100644
> > --- a/include/hw/cxl/cxl_host.h
> > +++ b/include/hw/cxl/cxl_host.h
> > @@ -14,8 +14,10 @@
> > #define CXL_HOST_H
> >
> > void cxl_machine_init(Object *obj, CXLState *state);
> > -void cxl_fmws_link_targets(CXLState *stat, Error **errp);
> > +void cxl_fmws_link_targets(Error **errp);
> > void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp);
> > +hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr);
> > +GSList *cxl_fmws_get_all_sorted(void);
> >
> > extern const MemoryRegionOps cfmws_ops;
> >
> > diff --git a/hw/acpi/cxl.c b/hw/acpi/cxl.c
> > index 9cd7905ea2..20806e5dd4 100644
> > --- a/hw/acpi/cxl.c
> > +++ b/hw/acpi/cxl.c
> > @@ -22,6 +22,7 @@
> > #include "hw/pci/pci_bridge.h"
> > #include "hw/pci/pci_host.h"
> > #include "hw/cxl/cxl.h"
> > +#include "hw/cxl/cxl_host.h"
> > #include "hw/mem/memory-device.h"
> > #include "hw/acpi/acpi.h"
> > #include "hw/acpi/aml-build.h"
> > @@ -135,56 +136,62 @@ static void cedt_build_chbs(GArray *table_data, PXBCXLDev *cxl)
> > * Interleave ways encoding in CXL 2.0 ECN: 3, 6, 12 and 16-way memory
> > * interleaving.
> > */
> > -static void cedt_build_cfmws(GArray *table_data, CXLState *cxls)
> > +static int cedt_build_cfmws(Object *obj, void *opaque)
>
> Too much unrelated indent updates in this function
I think that is because now the function only process one cfw, while it
used to process all. So the loop is no longer needed within this
function.
fan
>
>
> > {
> > - GList *it;
> > + struct CXLFixedWindow *fw;
> > + Aml *cedt = opaque;
> > + GArray *table_data = cedt->buf;
> > + int i;
> >
> > - for (it = cxls->fixed_windows; it; it = it->next) {
> > - CXLFixedWindow *fw = it->data;
> > - int i;
> > + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> > + return 0;
>
> Never reach here? It seems the caller has ensured passing TYPE_CXL_FMW obj.
>
>
>
>
> > + }
> > + fw = CXL_FMW(obj);
> >
> > - /* Type */
> > - build_append_int_noprefix(table_data, 1, 1);
> > + /* Type */
> > + build_append_int_noprefix(table_data, 1, 1);
> >
> > - /* Reserved */
> > - build_append_int_noprefix(table_data, 0, 1);
> > + /* Reserved */
> > + build_append_int_noprefix(table_data, 0, 1);
> >
> > - /* Record Length */
> > - build_append_int_noprefix(table_data, 36 + 4 * fw->num_targets, 2);
> > + /* Record Length */
> > + build_append_int_noprefix(table_data, 36 + 4 * fw->num_targets, 2);
> >
> > - /* Reserved */
> > - build_append_int_noprefix(table_data, 0, 4);
> > + /* Reserved */
> > + build_append_int_noprefix(table_data, 0, 4);
> >
> > - /* Base HPA */
> > - build_append_int_noprefix(table_data, fw->mr.addr, 8);
> > + /* Base HPA */
> > + build_append_int_noprefix(table_data, fw->mr.addr, 8);
> >
> > - /* Window Size */
> > - build_append_int_noprefix(table_data, fw->size, 8);
> > + /* Window Size */
> > + build_append_int_noprefix(table_data, fw->size, 8);
> >
> > - /* Host Bridge Interleave Ways */
> > - build_append_int_noprefix(table_data, fw->enc_int_ways, 1);
> > + /* Host Bridge Interleave Ways */
> > + build_append_int_noprefix(table_data, fw->enc_int_ways, 1);
> >
> > - /* Host Bridge Interleave Arithmetic */
> > - build_append_int_noprefix(table_data, 0, 1);
> > + /* Host Bridge Interleave Arithmetic */
> > + build_append_int_noprefix(table_data, 0, 1);
> >
> > - /* Reserved */
> > - build_append_int_noprefix(table_data, 0, 2);
> > + /* Reserved */
> > + build_append_int_noprefix(table_data, 0, 2);
> >
> > - /* Host Bridge Interleave Granularity */
> > - build_append_int_noprefix(table_data, fw->enc_int_gran, 4);
> > + /* Host Bridge Interleave Granularity */
> > + build_append_int_noprefix(table_data, fw->enc_int_gran, 4);
> >
> > - /* Window Restrictions */
> > - build_append_int_noprefix(table_data, 0x0f, 2); /* No restrictions */
> > + /* Window Restrictions */
> > + build_append_int_noprefix(table_data, 0x0f, 2);
> >
> > - /* QTG ID */
> > - build_append_int_noprefix(table_data, 0, 2);
> > + /* QTG ID */
> > + build_append_int_noprefix(table_data, 0, 2);
> >
> > - /* Host Bridge List (list of UIDs - currently bus_nr) */
> > - for (i = 0; i < fw->num_targets; i++) {
> > - g_assert(fw->target_hbs[i]);
> > - build_append_int_noprefix(table_data, PXB_DEV(fw->target_hbs[i])->bus_nr, 4);
> > - }
> > + /* Host Bridge List (list of UIDs - currently bus_nr) */
> > + for (i = 0; i < fw->num_targets; i++) {
> > + g_assert(fw->target_hbs[i]);
> > + build_append_int_noprefix(table_data,
> > + PXB_DEV(fw->target_hbs[i])->bus_nr, 4);
> > }
> > +
> > + return 0;
> > }
> >
> > static int cxl_foreach_pxb_hb(Object *obj, void *opaque)
> > @@ -202,6 +209,7 @@ void cxl_build_cedt(GArray *table_offsets, GArray *table_data,
> > BIOSLinker *linker, const char *oem_id,
> > const char *oem_table_id, CXLState *cxl_state)
> > {
> > + GSList *cfmws_list, *iter;
> > Aml *cedt;
> > AcpiTable table = { .sig = "CEDT", .rev = 1, .oem_id = oem_id,
> > .oem_table_id = oem_table_id };
> > @@ -213,7 +221,12 @@ void cxl_build_cedt(GArray *table_offsets, GArray *table_data,
> > /* reserve space for CEDT header */
> >
> > object_child_foreach_recursive(object_get_root(), cxl_foreach_pxb_hb, cedt);
> > - cedt_build_cfmws(cedt->buf, cxl_state);
> > +
> > + cfmws_list = cxl_fmws_get_all_sorted();
> > + for (iter = cfmws_list; iter; iter = iter->next) {
> > + cedt_build_cfmws(iter->data, cedt);
> > + }
> > + g_slist_free(cfmws_list);
> >
> > /* copy AML table into ACPI tables blob and patch header there */
> > g_array_append_vals(table_data, cedt->buf->data, cedt->buf->len);
> > diff --git a/hw/cxl/cxl-host-stubs.c b/hw/cxl/cxl-host-stubs.c
> > index cae4afcdde..13eb6bf6a4 100644
> > --- a/hw/cxl/cxl-host-stubs.c
> > +++ b/hw/cxl/cxl-host-stubs.c
> > @@ -8,8 +8,12 @@
> > #include "hw/cxl/cxl.h"
> > #include "hw/cxl/cxl_host.h"
> >
> > -void cxl_fmws_link_targets(CXLState *stat, Error **errp) {};
> > +void cxl_fmws_link_targets(Error **errp) {};
> > void cxl_machine_init(Object *obj, CXLState *state) {};
> > void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp) {};
> > +hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> > +{
> > + return base;
> > +};
> >
> > const MemoryRegionOps cfmws_ops;
> > diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
> > index b7aa429ddf..438f2920e1 100644
> > --- a/hw/cxl/cxl-host.c
> > +++ b/hw/cxl/cxl-host.c
> > @@ -22,12 +22,12 @@
> > #include "hw/pci/pcie_port.h"
> > #include "hw/pci-bridge/pci_expander_bridge.h"
> >
> > -static void cxl_fixed_memory_window_config(CXLState *cxl_state,
> > - CXLFixedMemoryWindowOptions *object,
> > +static void cxl_fixed_memory_window_config(CXLFixedMemoryWindowOptions *object,
> > int index, Error **errp)
> > {
> > ERRP_GUARD();
> > - g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
> > + DeviceState *dev = qdev_new(TYPE_CXL_FMW);
> > + CXLFixedWindow *fw = CXL_FMW(dev);
> > strList *target;
> > int i;
> >
> > @@ -67,35 +67,41 @@ static void cxl_fixed_memory_window_config(CXLState *cxl_state,
> > fw->targets[i] = g_strdup(target->value);
> > }
> >
> > - cxl_state->fixed_windows = g_list_append(cxl_state->fixed_windows,
> > - g_steal_pointer(&fw));
> > + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp);
> > +
> > + return;
> > }
> >
> > -void cxl_fmws_link_targets(CXLState *cxl_state, Error **errp)
> > +static int cxl_fmws_link(Object *obj, void *opaque)
> > {
> > - if (cxl_state && cxl_state->fixed_windows) {
> > - GList *it;
> > -
> > - for (it = cxl_state->fixed_windows; it; it = it->next) {
> > - CXLFixedWindow *fw = it->data;
> > - int i;
> > -
> > - for (i = 0; i < fw->num_targets; i++) {
> > - Object *o;
> > - bool ambig;
> > -
> > - o = object_resolve_path_type(fw->targets[i],
> > - TYPE_PXB_CXL_DEV,
> > - &ambig);
> > - if (!o) {
> > - error_setg(errp, "Could not resolve CXLFM target %s",
> > - fw->targets[i]);
> > - return;
> > - }
> > - fw->target_hbs[i] = PXB_CXL_DEV(o);
> > - }
> > + struct CXLFixedWindow *fw;
> > + int i;
> > +
> > + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> > + return 0;
> > + }
> > + fw = CXL_FMW(obj);
> > +
> > + for (i = 0; i < fw->num_targets; i++) {
> > + Object *o;
> > + bool ambig;
> > +
> > + o = object_resolve_path_type(fw->targets[i], TYPE_PXB_CXL_DEV,
> > + &ambig);
> > + if (!o) {
> > + error_setg(&error_fatal, "Could not resolve CXLFM target %s",
> > + fw->targets[i]);
> > + return 1;
> > }
> > + fw->target_hbs[i] = PXB_CXL_DEV(o);
> > }
> > + return 0;
> > +}
> > +
> > +void cxl_fmws_link_targets(Error **errp)
> > +{
> > + /* Order doesn't matter for this, so no need to build list */
> > + object_child_foreach_recursive(object_get_root(), cxl_fmws_link, NULL);
> > }
> >
> > static bool cxl_hdm_find_target(uint32_t *cache_mem, hwaddr addr,
> > @@ -335,7 +341,7 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
> > }
> >
> > for (it = cfmw_list, index = 0; it; it = it->next, index++) {
> > - cxl_fixed_memory_window_config(state, it->value, index, errp);
> > + cxl_fixed_memory_window_config(it->value, index, errp);
> > }
> > state->cfmw_list = cfmw_list;
> > }
> > @@ -373,3 +379,110 @@ void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp)
> > }
> > }
> > }
> > +
> > +struct cfmw_update_state {
> > + hwaddr base;
> > + hwaddr maxaddr;
> > +};
> > +
> > +static void cxl_fmws_update(Object *obj, void *opaque)
> > +{
> > + struct CXLFixedWindow *fw;
> > + struct cfmw_update_state *s = opaque;
> > +
> > + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
>
>
> Never reach here? It seems the caller has ensured passing TYPE_CXL_FMW obj.
>
>
> Thanks
> Zhijian
>
> > + return;
> > + }
> > +
> > + fw = CXL_FMW(obj);
> > + if (s->base + fw->size <= s->maxaddr) {
> > + fw->base = s->base;
> > + sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
> > + s->base += fw->size;
> > + }
> > +
> > + return;
> > +}
> > +
> > +static int cxl_fmws_find(Object *obj, void *opaque)
> > +{
> > + GSList **list = opaque;
> > +
> > + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> > + return 0;
> > + }
> > + *list = g_slist_prepend(*list, obj);
> > +
> > + return 0;
> > +}
> > +
> > +static GSList *cxl_fmws_get_all(void)
> > +{
> > + GSList *list = NULL;
> > +
> > + object_child_foreach_recursive(object_get_root(), cxl_fmws_find, &list);
> > +
> > + return list;
> > +}
> > +
> > +static gint cfmws_cmp(gconstpointer a, gconstpointer b, gpointer d)
> > +{
> > + const struct CXLFixedWindow *ap = a;
> > + const struct CXLFixedWindow *bp = b;
> > +
> > + return ap->index > bp->index;
> > +}
> > +
> > +GSList *cxl_fmws_get_all_sorted(void)
> > +{
> > + return g_slist_sort_with_data(cxl_fmws_get_all(), cfmws_cmp, NULL);
> > +}
> > +
> > +hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> > +{
> > + GSList *cfmws_list, *iter;
> > +
> > + struct cfmw_update_state cfmwss = {
> > + .base = base,
> > + .maxaddr = max_addr,
> > + };
> > + cfmws_list = cxl_fmws_get_all_sorted();
> > + for (iter = cfmws_list; iter; iter = iter->next) {
> > + cxl_fmws_update(iter->data, &cfmwss);
> > + }
> > + g_slist_free(cfmws_list);
> > +
> > + return cfmwss.base;
> > +}
> > +
> > +static void cxl_fmw_realize(DeviceState *dev, Error **errp)
> > +{
> > + CXLFixedWindow *fw = CXL_FMW(dev);
> > +
> > + memory_region_init_io(&fw->mr, OBJECT(dev), &cfmws_ops, fw,
> > + "cxl-fixed-memory-region", fw->size);
> > + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &fw->mr);
> > +}
> > +
> > +static void cxl_fmw_class_init(ObjectClass *klass, const void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > + dc->desc = "CXL Fixed Memory Window";
> > + dc->realize = cxl_fmw_realize;
> > + /* Reason - created by machines as tightly coupled to machine memory map */
> > + dc->user_creatable = false;
> > +}
> > +
> > +static const TypeInfo cxl_fmw_info = {
> > + .name = TYPE_CXL_FMW,
> > + .parent = TYPE_SYS_BUS_DEVICE,
> > + .instance_size = sizeof(CXLFixedWindow),
> > + .class_init = cxl_fmw_class_init,
> > +};
> > +
> > +static void cxl_host_register_types(void)
> > +{
> > + type_register_static(&cxl_fmw_info);
> > +}
> > +type_init(cxl_host_register_types)
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 70656157ca..9978398876 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -630,7 +630,7 @@ void pc_machine_done(Notifier *notifier, void *data)
> > &error_fatal);
> >
> > if (pcms->cxl_devices_state.is_enabled) {
> > - cxl_fmws_link_targets(&pcms->cxl_devices_state, &error_fatal);
> > + cxl_fmws_link_targets(&error_fatal);
> > }
> >
> > /* set the number of CPUs */
> > @@ -739,20 +739,28 @@ static uint64_t pc_get_cxl_range_start(PCMachineState *pcms)
> > return cxl_base;
> > }
> >
> > -static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
> > +static int cxl_get_fmw_end(Object *obj, void *opaque)
> > {
> > - uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
> > + struct CXLFixedWindow *fw;
> > + uint64_t *start = opaque;
> >
> > - if (pcms->cxl_devices_state.fixed_windows) {
> > - GList *it;
> > -
> > - start = ROUND_UP(start, 256 * MiB);
> > - for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
> > - CXLFixedWindow *fw = it->data;
> > - start += fw->size;
> > - }
> > + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> > + return 0;
> > }
> > + fw = CXL_FMW(obj);
> > +
> > + *start += fw->size;
> >
> > + return 0;
> > +}
> > +
> > +static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
> > +{
> > + uint64_t start = pc_get_cxl_range_start(pcms) + MiB;
> > +
> > + /* Ordering doesn't matter so no need to build a sorted list */
> > + object_child_foreach_recursive(object_get_root(), cxl_get_fmw_end,
> > + &start);
> > return start;
> > }
> >
> > @@ -954,23 +962,10 @@ void pc_memory_init(PCMachineState *pcms,
> > cxl_base = pc_get_cxl_range_start(pcms);
> > memory_region_init(mr, OBJECT(machine), "cxl_host_reg", cxl_size);
> > memory_region_add_subregion(system_memory, cxl_base, mr);
> > - cxl_resv_end = cxl_base + cxl_size;
> > - if (pcms->cxl_devices_state.fixed_windows) {
> > - hwaddr cxl_fmw_base;
> > - GList *it;
> > -
> > - cxl_fmw_base = ROUND_UP(cxl_base + cxl_size, 256 * MiB);
> > - for (it = pcms->cxl_devices_state.fixed_windows; it; it = it->next) {
> > - CXLFixedWindow *fw = it->data;
> > -
> > - fw->base = cxl_fmw_base;
> > - memory_region_init_io(&fw->mr, OBJECT(machine), &cfmws_ops, fw,
> > - "cxl-fixed-memory-region", fw->size);
> > - memory_region_add_subregion(system_memory, fw->base, &fw->mr);
> > - cxl_fmw_base += fw->size;
> > - cxl_resv_end = cxl_fmw_base;
> > - }
> > - }
> > + cxl_base = ROUND_UP(cxl_base + cxl_size, 256 * MiB);
> > +
> > + cxl_resv_end = cxl_fmws_set_memmap_and_update_mmio(cxl_base,
> > + maxphysaddr);
> > }
> >
> > /* Initialize PC system firmware */
--
Fan Ni (From gmail)
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL
2025-05-21 17:52 ` Jonathan Cameron
@ 2025-05-21 22:07 ` Itaru Kitayama
0 siblings, 0 replies; 24+ messages in thread
From: Itaru Kitayama @ 2025-05-21 22:07 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, Fan Ni, Peter Maydell, mst, linux-cxl, linuxarm,
qemu-arm, Yuquan Wang, Philippe Mathieu-Daudé
> On May 22, 2025, at 2:52, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>
> On Wed, 21 May 2025 16:38:10 +0900
> Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
>
>>> On May 19, 2025, at 21:54, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>>>
>>> On Thu, 15 May 2025 18:04:18 +0900
>>> Itaru Kitayama <itaru.kitayama@linux.dev> wrote:
>>>
>>>>> On May 13, 2025, at 20:14, Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>>>>>
>>>>> Add a single complex case for aarch64 virt machine.
>>>>> Given existing much more comprehensive tests for x86 cover the
>>>>> common functionality, a single test should be enough to verify
>>>>> that the aarch64 part continue to work.
>>>>>
>>>>> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>>>>> ---
>>>>> tests/qtest/cxl-test.c | 59 ++++++++++++++++++++++++++++++++---------
>>>>> tests/qtest/meson.build | 1 +
>>>>> 2 files changed, 47 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
>>>>> index a600331843..c7189d6222 100644
>>>>> --- a/tests/qtest/cxl-test.c
>>>>> +++ b/tests/qtest/cxl-test.c
>>>>> @@ -19,6 +19,12 @@
>>>>> "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
>>>>> "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
>>>>>
>>>>> +#define QEMU_VIRT_2PXB_CMD \
>>>>> + "-machine virt,cxl=on -cpu max " \
>>>>> + "-device pxb-cxl,id=cxl.0,bus=pcie.0,bus_nr=52 " \
>>>>> + "-device pxb-cxl,id=cxl.1,bus=pcie.0,bus_nr=53 " \
>>>>> + "-M cxl-fmw.0.targets.0=cxl.0,cxl-fmw.0.targets.1=cxl.1,cxl-fmw.0.size=4G "
>>>>> +
>>>>> #define QEMU_RP \
>>>>> "-device cxl-rp,id=rp0,bus=cxl.0,chassis=0,slot=0 "
>>>>>
>>>>> @@ -197,25 +203,52 @@ static void cxl_2pxb_4rp_4t3d(void)
>>>>> qtest_end();
>>>>> rmdir(tmpfs);
>>>>> }
>>>>> +
>>>>> +static void cxl_virt_2pxb_4rp_4t3d(void)
>>>>> +{
>>>>> + g_autoptr(GString) cmdline = g_string_new(NULL);
>>>>> + char template[] = "/tmp/cxl-test-XXXXXX";
>>>>> + const char *tmpfs;
>>>>> +
>>>>> + tmpfs = mkdtemp(template);
>>>>> +
>>>>> + g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
>>>>> + tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
>>>>> + tmpfs, tmpfs);
>>>>> +
>>>>> + qtest_start(cmdline->str);
>>>>> + qtest_end();
>>>>> + rmdir(tmpfs);
>>>>> +}
>>>>> #endif /* CONFIG_POSIX */
>>>>>
>>>>> int main(int argc, char **argv)
>>>>> {
>>>>> - g_test_init(&argc, &argv, NULL);
>>>>> + const char *arch = qtest_get_arch();
>>>>>
>>>>> - qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
>>>>> - qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
>>>>> - qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
>>>>> - qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
>>>>> - qtest_add_func("/pci/cxl/rp", cxl_root_port);
>>>>> - qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
>>>>> + g_test_init(&argc, &argv, NULL);
>>>>> + if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>>>>> + qtest_add_func("/pci/cxl/basic_hostbridge", cxl_basic_hb);
>>>>> + qtest_add_func("/pci/cxl/basic_pxb", cxl_basic_pxb);
>>>>> + qtest_add_func("/pci/cxl/pxb_with_window", cxl_pxb_with_window);
>>>>> + qtest_add_func("/pci/cxl/pxb_x2_with_window", cxl_2pxb_with_window);
>>>>> + qtest_add_func("/pci/cxl/rp", cxl_root_port);
>>>>> + qtest_add_func("/pci/cxl/rp_x2", cxl_2root_port);
>>>>> #ifdef CONFIG_POSIX
>>>>> - qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
>>>>> - qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
>>>>> - qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
>>>>> - qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
>>>>> - qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
>>>>> - qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4", cxl_2pxb_4rp_4t3d);
>>>>> + qtest_add_func("/pci/cxl/type3_device", cxl_t3d_deprecated);
>>>>> + qtest_add_func("/pci/cxl/type3_device_pmem", cxl_t3d_persistent);
>>>>> + qtest_add_func("/pci/cxl/type3_device_vmem", cxl_t3d_volatile);
>>>>> + qtest_add_func("/pci/cxl/type3_device_vmem_lsa", cxl_t3d_volatile_lsa);
>>>>> + qtest_add_func("/pci/cxl/rp_x2_type3_x2", cxl_1pxb_2rp_2t3d);
>>>>> + qtest_add_func("/pci/cxl/pxb_x2_root_port_x4_type3_x4",
>>>>> + cxl_2pxb_4rp_4t3d);
>>>>> #endif
>>>>> + } else if (strcmp(arch, "aarch64") == 0) {
>>>>> +#ifdef CONFIG_POSIX
>>>>> + qtest_add_func("/pci/cxl/virt/pxb_x2_root_port_x4_type3_x4",
>>>>> + cxl_virt_2pxb_4rp_4t3d);
>>>>> +#endif
>>>>> + }
>>>>> +
>>>>> return g_test_run();
>>>>> }
>>>>> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
>>>>> index 7daf619845..361000267a 100644
>>>>> --- a/tests/qtest/meson.build
>>>>> +++ b/tests/qtest/meson.build
>>>>> @@ -258,6 +258,7 @@ qtests_aarch64 = \
>>>>> (config_all_accel.has_key('CONFIG_TCG') and \
>>>>> config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
>>>>> (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
>>>>> + qtests_cxl + \
>>>>> ['arm-cpu-features',
>>>>> 'numa-test',
>>>>> 'boot-serial-test',
>>>>> --
>>>>> 2.43.0
>>>>>
>>>>
>>>> ~/projects/qemu/build$ meson test qtest-aarch64/cxl-test
>>>> ninja: Entering directory `/home/realm/projects/qemu/build'
>>>> [1/8] Generating qemu-version.h with a custom command (wrapped by meson to capture output)
>>>> 1/1 qemu:qtest+qtest-aarch64 / qtest-aarch64/cxl-test OK 0.17s 1 subtests passed
>>>>
>>>> Ok: 1
>>>> Expected Fail: 0
>>>> Fail: 0
>>>> Unexpected Pass: 0
>>>> Skipped: 0
>>>> Timeout: 0
>>>>
>>>> Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com <mailto:itaru.kitayama@fujitsu.com>>
>>>>
>>>> Jonathan, would you push your branch this series applied? I manually applied your series no issues though.
>>>
>>> I'm reluctant to push a 'normal' staging CXL tree whilst we have the TCG
>>> issue outstanding (which is in mainline).
>>> I can probably push one with a name that makes it clear we know it will
>>> crash under some circumstances though. I'll aim to get that done later this week.
>>>
>>> After talking to Richard Henderson I'm going to spin some images etc to
>>> make it easier for him to replicate that TCG issue.
>>
>> While QEMU (the kernel is built off of cxl branch) boots fine and lspci shows CXL devices as shown:
>>
>> root@localhost:~# lspci -mm
>> 00:00.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Host bridge" -p00 "Red Hat, Inc." "Device 1100"
>> 00:01.0 "SCSI storage controller" "Red Hat, Inc." "Virtio block device" -p00 "Red Hat, Inc." "Device 0002"
>> 00:02.0 "Ethernet controller" "Intel Corporation" "82540EM Gigabit Ethernet Controller" -r03 -p00 "Red Hat, Inc." "QEMU Virtual Machine"
>> 00:03.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Expander bridge" -p00 "Red Hat, Inc." "Device 1100"
>> 00:04.0 "Host bridge" "Red Hat, Inc." "QEMU PCIe Expander bridge" -p00 "Red Hat, Inc." "Device 1100"
>> 35:00.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
>> 35:01.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
>> 36:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a128" -p00 "" ""
>> 36:00.1 "Serial bus controller [0c0b]" "Huawei Technologies Co., Ltd." "Device a123" -p00 "Red Hat, Inc." "Device 1100"
>> 37:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
>> 37:01.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
>> 37:02.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
>> 37:03.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
>> 38:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
>> 3a:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
>> bf:00.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
>> bf:01.0 "PCI bridge" "Intel Corporation" "Device 7075" -p00 "Intel Corporation" "Device 0000"
>> c0:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a128" -p00 "" ""
>> c0:00.1 "Serial bus controller [0c0b]" "Huawei Technologies Co., Ltd." "Device a123" -p00 "Red Hat, Inc." "Device 1100"
>> c1:00.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
>> c1:01.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
>> c1:02.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
>> c1:03.0 "PCI bridge" "Huawei Technologies Co., Ltd." "Device a129" -p00 "" ""
>> c2:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100"
>> c4:00.0 "CXL" "Intel Corporation" "Device 0d93" -r01 -p10 "Red Hat, Inc." "Device 1100”
>>
>> but the cxl-list command takes 5-10 minutes to return the information. I’ll test with your qtest minimalist setup to see if the system is loaded. Am I seeing the TCG issue you mentioned?
>
> I don't think so. That should only matter if the memory is hotplugged in
> as normal system RAM.
>
>>
>> root@localhost:~# cxl list -M
>> [
>> {
>> "memdev":"mem10",
>> "ram_size":2147483648,
>> "ram_qos_class":42,
>> "serial":11,
>> "numa_node":0,
>> "host":"cxl_rcd.10",
>> "firmware_version":"mock fw v1 "
>
> So this seems to be with both some emulated devices from qemu command line
> and CXL test at the same time.
>
> I replicated a similar setup and not setting a significant delay
> (half a second maybe?) So not sure.
>
Good news. And I confirm that the cxl-list command returns a list quickly with older cxl/next kernel(s).
I am still in the process of bisecting trying to narrow down the bad commit.
Itaru.
> I did notice that there is a bug in qemu though which has surfaced
> for some reason in the kernel log (it dates all the way back to
> origin CXL support series). In
> include/hw/cxl/cxl_pci.h
> #define REG_LOC_DEVSEC_LENGTH is 0x24 and it should be 0x1C
>
> That results for me in a kernel log entry about a bar not being
> big enough for a huge offset (which is random data coming from
> somewhere in text I think).
>
> Seems unlikely to trigger your issue, but you never know!
>
> Jonathan
>
>
>> },
>> {
>> "memdev":"mem5",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":6,
>> "numa_node":1,
>> "host":"cxl_mem.5",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem1",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":2,
>> "numa_node":1,
>> "host":"cxl_mem.1",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem6",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":8,
>> "numa_node":1,
>> "host":"cxl_mem.7",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem3",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":4,
>> "numa_node":1,
>> "host":"cxl_mem.3",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem4",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":5,
>> "numa_node":0,
>> "host":"cxl_mem.4",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem0",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":1,
>> "numa_node":0,
>> "host":"cxl_mem.0",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem2",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":3,
>> "numa_node":0,
>> "host":"cxl_mem.2",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem7",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":7,
>> "numa_node":0,
>> "host":"cxl_mem.6",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem8",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":9,
>> "numa_node":0,
>> "host":"cxl_mem.8",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem9",
>> "pmem_size":1073741824,
>> "pmem_qos_class":42,
>> "ram_size":1073741824,
>> "ram_qos_class":42,
>> "serial":10,
>> "numa_node":1,
>> "host":"cxl_mem.9",
>> "firmware_version":"mock fw v1 "
>> },
>> {
>> "memdev":"mem12",
>> "ram_size":268435456,
>> "serial":0,
>> "host":"0000:c4:00.0",
>> "firmware_version":"BWFW VERSION 00"
>> },
>> {
>> "memdev":"mem11",
>> "ram_size":268435456,
>> "serial":0,
>> "host":"0000:c2:00.0",
>> "firmware_version":"BWFW VERSION 00"
>> },
>> {
>> "memdev":"mem14",
>> "pmem_size":268435456,
>> "serial":0,
>> "host":"0000:3a:00.0",
>> "firmware_version":"BWFW VERSION 00"
>> },
>> {
>> "memdev":"mem13",
>> "pmem_size":268435456,
>> "serial":0,
>> "host":"0000:38:00.0",
>> "firmware_version":"BWFW VERSION 00"
>> }
>> ]
>>
>> Getting this shouldn’t take minutes, even with the emulator I think.
>>
>> Itaru.
>>
>>>
>>> Thanks for reviews.
>>>
>>> Jonathan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices.
2025-05-16 5:44 ` Zhijian Li (Fujitsu)
2025-05-21 17:54 ` Fan Ni
@ 2025-05-27 16:04 ` Jonathan Cameron
1 sibling, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-27 16:04 UTC (permalink / raw)
To: Zhijian Li (Fujitsu)
Cc: qemu-devel@nongnu.org, Fan Ni, Peter Maydell, mst@redhat.com,
linux-cxl@vger.kernel.org, linuxarm@huawei.com,
qemu-arm@nongnu.org, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
On Fri, 16 May 2025 05:44:34 +0000
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> wrote:
> On 13/05/2025 19:14, Jonathan Cameron via wrote:
> > Previously these somewhat device like structures were tracked using a list
> > in the CXLState in each machine. This is proving restrictive in a few
> > cases where we need to iterate through these without being aware of the
> > machine type. Just make them sysbus devices.
> Make sense.
>
> Minor comments inline
> > diff --git a/hw/acpi/cxl.c b/hw/acpi/cxl.c
> > index 9cd7905ea2..20806e5dd4 100644
> > --- a/hw/acpi/cxl.c
> > +++ b/hw/acpi/cxl.c
> > @@ -22,6 +22,7 @@
> > #include "hw/pci/pci_bridge.h"
> > #include "hw/pci/pci_host.h"
> > #include "hw/cxl/cxl.h"
> > +#include "hw/cxl/cxl_host.h"
> > #include "hw/mem/memory-device.h"
> > #include "hw/acpi/acpi.h"
> > #include "hw/acpi/aml-build.h"
> > @@ -135,56 +136,62 @@ static void cedt_build_chbs(GArray *table_data, PXBCXLDev *cxl)
> > * Interleave ways encoding in CXL 2.0 ECN: 3, 6, 12 and 16-way memory
> > * interleaving.
> > */
> > -static void cedt_build_cfmws(GArray *table_data, CXLState *cxls)
> > +static int cedt_build_cfmws(Object *obj, void *opaque)
>
> Too much unrelated indent updates in this function
Fan addressed this. It's due to the loop now being external to this
call.
>
>
> > {
> > - GList *it;
> > + struct CXLFixedWindow *fw;
> > + Aml *cedt = opaque;
> > + GArray *table_data = cedt->buf;
> > + int i;
> >
> > - for (it = cxls->fixed_windows; it; it = it->next) {
> > - CXLFixedWindow *fw = it->data;
> > - int i;
> > + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
> > + return 0;
>
> Never reach here? It seems the caller has ensured passing TYPE_CXL_FMW obj.
Excellent point. Further than that, now the iterator is gone
I can just pass in correctly typed pointers from the caller
which is a nice to have as well!
>
>
>
>
> > + }
> > + fw = CXL_FMW(obj);
> >
> > - /* Type */
> > - build_append_int_noprefix(table_data, 1, 1);
> > + /* Type */
> > + build_append_int_noprefix(table_data, 1, 1);
> >
> > - /* Reserved */
> > - build_append_int_noprefix(table_data, 0, 1);
> > + /* Reserved */
> > + build_append_int_noprefix(table_data, 0, 1);
> >
> > - /* Record Length */
> > - build_append_int_noprefix(table_data, 36 + 4 * fw->num_targets, 2);
> > + /* Record Length */
> > + build_append_int_noprefix(table_data, 36 + 4 * fw->num_targets, 2);
> >
> > - /* Reserved */
> > - build_append_int_noprefix(table_data, 0, 4);
> > + /* Reserved */
> > + build_append_int_noprefix(table_data, 0, 4);
> >
> > - /* Base HPA */
> > - build_append_int_noprefix(table_data, fw->mr.addr, 8);
> > + /* Base HPA */
> > + build_append_int_noprefix(table_data, fw->mr.addr, 8);
> >
> > - /* Window Size */
> > - build_append_int_noprefix(table_data, fw->size, 8);
> > + /* Window Size */
> > + build_append_int_noprefix(table_data, fw->size, 8);
> >
> > - /* Host Bridge Interleave Ways */
> > - build_append_int_noprefix(table_data, fw->enc_int_ways, 1);
> > + /* Host Bridge Interleave Ways */
> > + build_append_int_noprefix(table_data, fw->enc_int_ways, 1);
> >
> > - /* Host Bridge Interleave Arithmetic */
> > - build_append_int_noprefix(table_data, 0, 1);
> > + /* Host Bridge Interleave Arithmetic */
> > + build_append_int_noprefix(table_data, 0, 1);
> >
> > - /* Reserved */
> > - build_append_int_noprefix(table_data, 0, 2);
> > + /* Reserved */
> > + build_append_int_noprefix(table_data, 0, 2);
> >
> > - /* Host Bridge Interleave Granularity */
> > - build_append_int_noprefix(table_data, fw->enc_int_gran, 4);
> > + /* Host Bridge Interleave Granularity */
> > + build_append_int_noprefix(table_data, fw->enc_int_gran, 4);
> >
> > - /* Window Restrictions */
> > - build_append_int_noprefix(table_data, 0x0f, 2); /* No restrictions */
> > + /* Window Restrictions */
> > + build_append_int_noprefix(table_data, 0x0f, 2);
> >
> > - /* QTG ID */
> > - build_append_int_noprefix(table_data, 0, 2);
> > + /* QTG ID */
> > + build_append_int_noprefix(table_data, 0, 2);
> >
> > - /* Host Bridge List (list of UIDs - currently bus_nr) */
> > - for (i = 0; i < fw->num_targets; i++) {
> > - g_assert(fw->target_hbs[i]);
> > - build_append_int_noprefix(table_data, PXB_DEV(fw->target_hbs[i])->bus_nr, 4);
> > - }
> > + /* Host Bridge List (list of UIDs - currently bus_nr) */
> > + for (i = 0; i < fw->num_targets; i++) {
> > + g_assert(fw->target_hbs[i]);
> > + build_append_int_noprefix(table_data,
> > + PXB_DEV(fw->target_hbs[i])->bus_nr, 4);
> > }
> > +
> > + return 0;
> > }
> > index cae4afcdde..13eb6bf6a4 100644
> > --- a/hw/cxl/cxl-host-stubs.c
> > +++ b/hw/cxl/cxl-host-stubs.c
> > @@ -8,8 +8,12 @@
> > +
> > +struct cfmw_update_state {
> > + hwaddr base;
> > + hwaddr maxaddr;
> > +};
> > +
> > +static void cxl_fmws_update(Object *obj, void *opaque)
> > +{
> > + struct CXLFixedWindow *fw;
> > + struct cfmw_update_state *s = opaque;
> > +
> > + if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
>
>
> Never reach here? It seems the caller has ensured passing TYPE_CXL_FMW obj.
Also a good point. Here as well I can simply pass the
correct type of pointer in for this and hwaddr *base, hwaddr max_addr
removing the need for cfmw_update_state()
That is all stale infrastructure from before I changed this handling
to force the device order.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup.
2025-05-16 5:50 ` Zhijian Li (Fujitsu)
@ 2025-05-27 16:28 ` Jonathan Cameron
0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2025-05-27 16:28 UTC (permalink / raw)
To: Zhijian Li (Fujitsu)
Cc: qemu-devel@nongnu.org, Fan Ni, Peter Maydell, mst@redhat.com,
linux-cxl@vger.kernel.org, linuxarm@huawei.com,
qemu-arm@nongnu.org, Yuquan Wang, Itaru Kitayama,
Philippe Mathieu-Daudé
On Fri, 16 May 2025 05:50:59 +0000
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> wrote:
> On 13/05/2025 19:14, Jonathan Cameron via wrote:
> >
> > +hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr)
> > +{
> > + GSList *cfmws_list, *iter;
> > +
> > + struct cfmw_update_state cfmwss = {
> > + .base = base,
> > + .maxaddr = max_addr,
> > + .update_mmio = false,
> > + };
> > + cfmws_list = cxl_fmws_get_all_sorted();
> > + for (iter = cfmws_list; iter; iter = iter->next) {
> > + cxl_fmws_update(iter->data, &cfmwss);
> > + }
> > + g_slist_free(cfmws_list);
> > +
> > + return cfmwss.base;
> > +}
> > +
> > hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> > {
> > GSList *cfmws_list, *iter;
> > @@ -445,6 +466,7 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> > struct cfmw_update_state cfmwss = {
> > .base = base,
> > .maxaddr = max_addr,
> > + .update_mmio = true,
> > };
> > cfmws_list = cxl_fmws_get_all_sorted();
> > for (iter = cfmws_list; iter; iter = iter->next) {
> > @@ -455,6 +477,26 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> > return cfmwss.base;
> > }
> >
>
> It seem we can share most of the code in cxl_fmws_set_memmap_and_update_mmio() and cxl_fmws_set_memmap()
> In addition, we can drop the cfmw_update_state::update_mmio if there is no other users. Just pass it
> to cxl_fmws_update() directly.
I'd like to keep the 'documentation' aspect of the function names, but
can reduce duplication with a do_cxl_fmws_set_mmemap_and_update_mmio() function that
has a shared implementation and takes the extra parameter as you suggestion.
Thanks
Jonathan
>
>
> Thanks
> Zhijian
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow
2025-05-13 11:14 ` [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow Jonathan Cameron
2025-05-16 5:44 ` Zhijian Li (Fujitsu)
2025-05-21 16:59 ` Fan Ni
@ 2025-08-08 8:29 ` wangyuquan
2025-08-08 8:57 ` Yuquan Wang
3 siblings, 0 replies; 24+ messages in thread
From: wangyuquan @ 2025-08-08 8:29 UTC (permalink / raw)
To: tangtao1634, qemu-devel, Fan Ni, Peter Maydell, mst
Cc: Jonathan Cameron, linux-cxl, linuxarm, qemu-arm, Yuquan Wang,
Itaru Kitayama, Philippe Mathieu-Daudé
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
To enable these to be found in a fixed order, that order needs
to be known. This will later be used to sort a list of these
structures that address map and ACPI table entries are predictable.
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
include/hw/cxl/cxl.h | 1 +
hw/cxl/cxl-host.c | 9 ++++++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 75e47b6864..b2bcce7ed6 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -27,6 +27,7 @@
typedef struct PXBCXLDev PXBCXLDev;
typedef struct CXLFixedWindow {
+ int index;
uint64_t size;
char **targets;
PXBCXLDev *target_hbs[16];
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index e010163174..b7aa429ddf 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -24,13 +24,15 @@
static void cxl_fixed_memory_window_config(CXLState *cxl_state,
CXLFixedMemoryWindowOptions *object,
- Error **errp)
+ int index, Error **errp)
{
ERRP_GUARD();
g_autofree CXLFixedWindow *fw = g_malloc0(sizeof(*fw));
strList *target;
int i;
+ fw->index = index;
+
for (target = object->targets; target; target = target->next) {
fw->num_targets++;
}
@@ -325,14 +327,15 @@ static void machine_set_cfmw(Object *obj, Visitor *v, const char *name,
CXLState *state = opaque;
CXLFixedMemoryWindowOptionsList *cfmw_list = NULL;
CXLFixedMemoryWindowOptionsList *it;
+ int index;
visit_type_CXLFixedMemoryWindowOptionsList(v, name, &cfmw_list, errp);
if (!cfmw_list) {
return;
}
- for (it = cfmw_list; it; it = it->next) {
- cxl_fixed_memory_window_config(state, it->value, errp);
+ for (it = cfmw_list, index = 0; it; it = it->next, index++) {
+ cxl_fixed_memory_window_config(state, it->value, index, errp);
}
state->cfmw_list = cfmw_list;
}
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow
2025-05-13 11:14 ` [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow Jonathan Cameron
` (2 preceding siblings ...)
2025-08-08 8:29 ` wangyuquan
@ 2025-08-08 8:57 ` Yuquan Wang
3 siblings, 0 replies; 24+ messages in thread
From: Yuquan Wang @ 2025-08-08 8:57 UTC (permalink / raw)
To: linux-cxl
Sorry for disturbing! I tried to test send more than 10 emails on my smtp server！
> -----原始邮件-----
> 发件人: wangyuquan <wangyuquan1236@phytium.com.cn>
> 发送时间:2025-08-08 16:29:57 (星期五)
> 收件人: tangtao1634@phytium.com.cn, qemu-devel@nongnu.org, "Fan Ni" <fan.ni@samsung.com>, "Peter Maydell" <peter.maydell@linaro.org>, mst@redhat.com
> 抄送: "Jonathan Cameron" <Jonathan.Cameron@huawei.com>, linux-cxl@vger.kernel.org, linuxarm@huawei.com, qemu-arm@nongnu.org, "Yuquan Wang" <wangyuquan1236@phytium.com.cn>, "Itaru Kitayama" <itaru.kitayama@linux.dev>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
> 主题: [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow
信息安全声明：本邮件包含信息归发件人所在组织所有,发件人所在组织对该邮件拥有所有权利。请接收者注意保密,未经发件人书面许可,不得向任何第三方组织和个人透露本邮件所含信息。
Information Security Notice: The information contained in this mail is solely property of the sender's organization.This mail communication is confidential.Recipients named above are obligated to maintain secrecy and are not permitted to disclose the contents of this communication to others.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2025-08-08 8:57 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-13 11:14 [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow Jonathan Cameron
2025-05-16 5:44 ` Zhijian Li (Fujitsu)
2025-05-21 16:59 ` Fan Ni
2025-08-08 8:29 ` wangyuquan
2025-08-08 8:57 ` Yuquan Wang
2025-05-13 11:14 ` [PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices Jonathan Cameron
2025-05-16 5:44 ` Zhijian Li (Fujitsu)
2025-05-21 17:54 ` Fan Ni
2025-05-27 16:04 ` Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup Jonathan Cameron
2025-05-16 5:50 ` Zhijian Li (Fujitsu)
2025-05-27 16:28 ` Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 4/5] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl Jonathan Cameron
2025-05-13 11:14 ` [PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL Jonathan Cameron
2025-05-15 9:04 ` Itaru Kitayama
2025-05-19 12:54 ` Jonathan Cameron
2025-05-20 20:38 ` Itaru Kitayama
2025-05-21 7:38 ` Itaru Kitayama
2025-05-21 17:52 ` Jonathan Cameron
2025-05-21 22:07 ` Itaru Kitayama
2025-05-16 2:30 ` [PATCH v13 0/5] arm/virt: CXL support via pxb_cxl Itaru Kitayama
2025-05-16 6:34 ` Itaru Kitayama
2025-05-20 17:31 ` 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).