[PATCH v15 0/4] arm/virt: CXL support via pxb_cxl

[PATCH v15 0/4] arm/virt: CXL support via pxb_cxl

[Jonathan Cameron via]

v15:
  - Split the address map calculations and mmio setup into separate
    functions in patch 2, allowing v14 patch 3 to be dropped as not
    x86 and arm make the same calls.  Note I felt this was a sufficient
    change to trigger dropping tags. (Zhijian Li)
  - A few other minor tweaks.
  - TLB issue mentioned in v14 now fixed upstream so dropped reference
    in this cover letter.

Thanks to Itaru Kitayama and Zhijian Li for testing + reviews.

Updated 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>.  Will update shortly with this
series.

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!)

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.

Also, we are kicking off some work on DCD virtualization, particularly to
support inter-host shared memory being presented up into a VM. That
will need upstream support on arm64 as it is built on top of the existing
CXL emulation to avoid the need for a separate guest software stack.

Note this is TCG only - it is possible to support limited use with KVM but
that needs additional patches not yet ready for upstream.  The challenge
is interleave - and the solution is don't interleave if you want to run
with KVM.

Jonathan Cameron (4):
  hw/cxl-host: Add an index field to CXLFixedMemoryWindow
  hw/cxl: Make the CXL fixed memory windows devices.
  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      |   5 +-
 include/hw/cxl/cxl_host.h |   5 +-
 hw/acpi/cxl.c             |  76 +++++++++--------
 hw/arm/virt-acpi-build.c  |  34 ++++++++
 hw/arm/virt.c             |  29 +++++++
 hw/cxl/cxl-host-stubs.c   |   7 +-
 hw/cxl/cxl-host.c         | 170 +++++++++++++++++++++++++++++++-------
 hw/i386/pc.c              |  50 +++++------
 tests/qtest/cxl-test.c    |  59 ++++++++++---
 tests/qtest/meson.build   |   1 +
 11 files changed, 330 insertions(+), 110 deletions(-)

-- 
2.48.1

[PATCH v15 1/4] hw/cxl-host: Add an index field to CXLFixedMemoryWindow

[Jonathan Cameron via]

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 so that address
map and ACPI table entries are predictable.

Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
Reviewed-by: Fan Ni <fan.ni@samsung.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

---
v14: Picked up tags. Thanks!
---
 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.48.1

[PATCH v15 2/4] hw/cxl: Make the CXL fixed memory windows devices.

[Jonathan Cameron via]

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.

In the setup path for the memory map in pc_memory_init() split the
operations into two calls. The first, cxl_fmws_set_mmemap(), loops over
fixed memory windows in order and assigns their addresses.  The second,
cxl_fmws_update_mmio() actually sets up the mmio for each window.
This is obviously less efficient than a single loop but this split design
is needed to put the logic in two different places in the arm64 support
and it is not a hot enough path to justify an x86 only implementation.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v15: Drop the left over GList *fixed_windows from CXLState.
     Spit cxl_fmws_set_memmap_and_update_mmio() in to remove
     need to do this in next patch. Patch description now has a
     paragraph justifying this design decision. (thanks Zhijian Li
     for suggesting it).
     Dropped tags as this is a non trivial change.

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      |   4 +-
 include/hw/cxl/cxl_host.h |   5 +-
 hw/acpi/cxl.c             |  76 +++++++++---------
 hw/cxl/cxl-host-stubs.c   |   7 +-
 hw/cxl/cxl-host.c         | 163 +++++++++++++++++++++++++++++++-------
 hw/i386/pc.c              |  50 +++++-------
 6 files changed, 210 insertions(+), 95 deletions(-)

diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index b2bcce7ed6..de66ab8c35 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,12 +39,13 @@ 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;
     MemoryRegion host_mr;
     unsigned int next_mr_idx;
-    GList *fixed_windows;
     CXLFixedMemoryWindowOptionsList *cfmw_list;
 } CXLState;
 
diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
index c9bc9c7c50..cd3c368c86 100644
--- a/include/hw/cxl/cxl_host.h
+++ b/include/hw/cxl/cxl_host.h
@@ -14,8 +14,11 @@
 #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(hwaddr base, hwaddr max_addr);
+void cxl_fmws_update_mmio(void);
+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..75d5b30bb8 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,55 +136,52 @@ 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 void cedt_build_cfmws(CXLFixedWindow *fw, Aml *cedt)
 {
-    GList *it;
+    GArray *table_data = cedt->buf;
+    int i;
 
-    for (it = cxls->fixed_windows; it; it = it->next) {
-        CXLFixedWindow *fw = it->data;
-        int i;
-
-        /* 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);
     }
 }
 
@@ -202,6 +200,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 +212,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(CXL_FMW(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..c015baac81 100644
--- a/hw/cxl/cxl-host-stubs.c
+++ b/hw/cxl/cxl-host-stubs.c
@@ -8,8 +8,13 @@
 #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(hwaddr base, hwaddr max_addr)
+{
+    return base;
+};
+void cxl_fmws_update_mmio(void) {};
 
 const MemoryRegionOps cfmws_ops;
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index b7aa429ddf..5239555f6c 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,39 @@ 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);
 }
 
-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 +339,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 +377,106 @@ void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp)
         }
     }
 }
+
+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);
+}
+
+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);
+}
+
+hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr)
+{
+    GSList *cfmws_list, *iter;
+    CXLFixedWindow *fw;
+
+    cfmws_list = cxl_fmws_get_all_sorted();
+    for (iter = cfmws_list; iter; iter = iter->next) {
+        fw = CXL_FMW(iter->data);
+        if (base + fw->size <= max_addr) {
+            fw->base = base;
+            base += fw->size;
+        }
+    }
+    g_slist_free(cfmws_list);
+
+    return 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 b211633575..860346d6b7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -609,7 +609,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 */
@@ -718,20 +718,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;
 }
 
@@ -933,23 +941,9 @@ 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(cxl_base, maxphysaddr);
+        cxl_fmws_update_mmio();
     }
 
     /* Initialize PC system firmware */
-- 
2.48.1

[PATCH v15 3/4] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl

[Jonathan Cameron via]

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>

---
v15: No changes.
---
 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.48.1

[PATCH v15 4/4] qtest/cxl: Add aarch64 virt test for CXL

[Jonathan Cameron via]

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>

---
v15: Dropped tags due to changes in patches 2 and 3.
---
 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 8ad849054f..42e927b32a 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -261,6 +261,7 @@ qtests_aarch64 = \
    config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
   (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
   (config_all_devices.has_key('CONFIG_NPCM8XX') ? qtests_npcm8xx : []) + \
+  qtests_cxl +                                                                                  \
   ['arm-cpu-features',
    'numa-test',
    'boot-serial-test',
-- 
2.48.1

Re: [PATCH v15 0/4] arm/virt: CXL support via pxb_cxl

[Peter Maydell]

On Thu, 12 Jun 2025 at 14:43, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> v15:
>   - Split the address map calculations and mmio setup into separate
>     functions in patch 2, allowing v14 patch 3 to be dropped as not
>     x86 and arm make the same calls.  Note I felt this was a sufficient
>     change to trigger dropping tags. (Zhijian Li)
>   - A few other minor tweaks.
>   - TLB issue mentioned in v14 now fixed upstream so dropped reference
>     in this cover letter.
>
> Thanks to Itaru Kitayama and Zhijian Li for testing + reviews.

Hi Jonathan -- I forget where we are on this. Do you think this
series is OK to go upstream at this point, or is there still
stuff you're iterating on here ?  (Put another way, should I
review it and consider queueing it?)

thanks
-- PMM

Re: [PATCH v15 0/4] arm/virt: CXL support via pxb_cxl

[Jonathan Cameron via]

On Thu, 12 Jun 2025 17:04:32 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 12 Jun 2025 at 14:43, Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > v15:
> >   - Split the address map calculations and mmio setup into separate
> >     functions in patch 2, allowing v14 patch 3 to be dropped as not
> >     x86 and arm make the same calls.  Note I felt this was a sufficient
> >     change to trigger dropping tags. (Zhijian Li)
> >   - A few other minor tweaks.
> >   - TLB issue mentioned in v14 now fixed upstream so dropped reference
> >     in this cover letter.
> >
> > Thanks to Itaru Kitayama and Zhijian Li for testing + reviews.  
> 
> Hi Jonathan -- I forget where we are on this. Do you think this
> series is OK to go upstream at this point, or is there still
> stuff you're iterating on here ?  (Put another way, should I
> review it and consider queueing it?)

Hi Peter,

Please do review + consider queuing it.  The recent changes have
just been tidying up based on reviews and before that it was just
the switch over to sysbus devices for the CXL Fixed Memory Windows.
That was mainly because I need that for other stuff and I think
you suggested it a few years back so I thought it might make you
happier with this :)

Thanks,

Jonathan

> 
> thanks
> -- PMM

Re: [PATCH v15 4/4] qtest/cxl: Add aarch64 virt test for CXL

[Itaru Kitayama]

On Thu, Jun 12, 2025 at 02:43:38PM +0100, Jonathan Cameron 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>
> 
> ---
> v15: Dropped tags due to changes in patches 2 and 3.
> ---
>  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 8ad849054f..42e927b32a 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -261,6 +261,7 @@ qtests_aarch64 = \
>     config_all_devices.has_key('CONFIG_TPM_TIS_I2C') ? ['tpm-tis-i2c-test'] : []) + \
>    (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed64 : []) + \
>    (config_all_devices.has_key('CONFIG_NPCM8XX') ? qtests_npcm8xx : []) + \
> +  qtests_cxl +                                                                                  \
>    ['arm-cpu-features',
>     'numa-test',
>     'boot-serial-test',

V15 is applied cleanly using b4 on top of today's master.

$ 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.21s   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>

Thanks,
Itaru

> -- 
> 2.48.1
> 

Re: [PATCH v15 2/4] hw/cxl: Make the CXL fixed memory windows devices.

[Zhijian Li (Fujitsu) via]

On 12/06/2025 21:43, Jonathan Cameron 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.
> 
> 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.
> 
> In the setup path for the memory map in pc_memory_init() split the
> operations into two calls. The first, cxl_fmws_set_mmemap(), loops over
> fixed memory windows in order and assigns their addresses.  The second,
> cxl_fmws_update_mmio() actually sets up the mmio for each window.
> This is obviously less efficient than a single loop but this split design
> is needed to put the logic in two different places in the arm64 support
> and it is not a hot enough path to justify an x86 only implementation.
> 
> Signed-off-by: Jonathan Cameron<Jonathan.Cameron@huawei.com>


Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
Tested-by: Li Zhijian <lizhijian@fujitsu.com>

> ---
> v15: Drop the left over GList *fixed_windows from CXLState.
>       Spit cxl_fmws_set_memmap_and_update_mmio() in to remove
>       need to do this in next patch. Patch description now has a
>       paragraph justifying this design decision. (thanks Zhijian Li
>       for suggesting it).
>       Dropped tags as this is a non trivial change.

[snip]

Re: [PATCH v15 1/4] hw/cxl-host: Add an index field to CXLFixedMemoryWindow

[Zhijian Li (Fujitsu) via]

On 12/06/2025 21:43, 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 so that address
> map and ACPI table entries are predictable.
> 
> Reviewed-by: Li Zhijian<lizhijian@fujitsu.com>
> Reviewed-by: Fan Ni<fan.ni@samsung.com>
> Signed-off-by: Jonathan Cameron<Jonathan.Cameron@huawei.com>

Tested-by: Li Zhijian <lizhijian@fujitsu.com>

Re: [PATCH v15 0/4] arm/virt: CXL support via pxb_cxl

[Itaru Kitayama]

On Thu, Jun 12, 2025 at 02:43:34PM +0100, Jonathan Cameron wrote:
> v15:
>   - Split the address map calculations and mmio setup into separate
>     functions in patch 2, allowing v14 patch 3 to be dropped as not
>     x86 and arm make the same calls.  Note I felt this was a sufficient
>     change to trigger dropping tags. (Zhijian Li)
>   - A few other minor tweaks.
>   - TLB issue mentioned in v14 now fixed upstream so dropped reference
>     in this cover letter.
> 
> Thanks to Itaru Kitayama and Zhijian Li for testing + reviews.
> 
> Updated 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>.  Will update shortly with this
> series.
> 
> 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!)
> 
> 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.
> 
> Also, we are kicking off some work on DCD virtualization, particularly to
> support inter-host shared memory being presented up into a VM. That
> will need upstream support on arm64 as it is built on top of the existing
> CXL emulation to avoid the need for a separate guest software stack.
> 
> Note this is TCG only - it is possible to support limited use with KVM but
> that needs additional patches not yet ready for upstream.  The challenge
> is interleave - and the solution is don't interleave if you want to run
> with KVM.

One of the ndctl:cxl tests fails (other tests ran ok):

# meson test cxl-region-sysfs.sh
ninja: Entering directory `/root/ndctl/build'
[1/55] Generating version.h with a custom command
[  706.564783][ T2080] calling  cxl_port_init+0x0/0xfe0 [cxl_port] @ 2080
[  706.566861][ T2080] initcall cxl_port_init+0x0/0xfe0 [cxl_port] returned 0 after 1735 usecs
[  706.586457][ T2080] calling  cxl_acpi_init+0x0/0xfe0 [cxl_acpi] @ 2080
[  706.625690][ T2080] probe of port1 returned 0 after 25381 usecs
[  706.626634][ T2080]  pci0000:bf: host supports CXL
[  706.653573][ T2080] probe of port2 returned 0 after 25631 usecs
[  706.655164][ T2080]  pci0000:35: host supports CXL
[  706.662409][ T2080] probe of ACPI0017:00 returned 0 after 74464 usecs
[  706.663150][ T2080] initcall cxl_acpi_init+0x0/0xfe0 [cxl_acpi] returned 0 after 76306 usecs
[  706.690482][ T2080] calling  cxl_pmem_init+0x0/0xfd0 [cxl_pmem] @ 2080
[  706.695324][ T2080] probe of ndbus0 returned 0 after 1496 usecs
[  706.699217][ T2080] probe of nvdimm-bridge0 returned 0 after 6705 usecs
[  706.702372][ T2080] initcall cxl_pmem_init+0x0/0xfd0 [cxl_pmem] returned 0 after 11576 usecs
[  706.717668][ T2080] calling  cxl_mem_driver_init+0x0/0xfe0 [cxl_mem] @ 2080
[  706.758561][ T2080] probe of port3 returned 0 after 34188 usecs
[  706.767080][ T2080] cxl_nvdimm pmem11: GPF: could not set dirty shutdown state
[  706.779392][ T2080] probe of nmem0 returned 0 after 1083 usecs
[  706.782181][ T2080] probe of pmem11 returned 0 after 15516 usecs
[  706.826941][ T2080] probe of endpoint4 returned 0 after 42630 usecs
[  706.827987][ T2080] probe of mem11 returned 0 after 108475 usecs
[  706.878052][ T2080] probe of port5 returned 0 after 41354 usecs
[  706.938260][ T2080] probe of endpoint6 returned 0 after 46831 usecs
[  706.939223][ T2080] probe of mem12 returned 0 after 105104 usecs
[  706.994611][ T2080] probe of endpoint7 returned 0 after 49337 usecs
[  706.995790][ T2080] probe of mem13 returned 0 after 53632 usecs
[  707.004334][ T2080] cxl_nvdimm pmem14: GPF: could not set dirty shutdown state
[  707.017782][ T2080] probe of nmem1 returned 0 after 1115 usecs
[  707.019324][ T2080] probe of pmem14 returned 0 after 14920 usecs
[  707.072148][ T2080] probe of endpoint8 returned 0 after 50887 usecs
[  707.073367][ T2080] probe of mem14 returned 0 after 71279 usecs
[  707.079062][ T2080] initcall cxl_mem_driver_init+0x0/0xfe0 [cxl_mem] returned 0 after 361073 usecs
[  707.111533][ T2080] calling  cxl_test_init+0x0/0xc88 [cxl_test] @ 2080
[  708.001403][ T2080] platform cxl_host_bridge.0: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
[  708.002032][ T2080] platform cxl_host_bridge.1: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
[  708.010988][ T2080] platform cxl_host_bridge.2: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
[  708.011963][ T2080] platform cxl_host_bridge.3: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
[  708.034604][ T2080] platform cxl_host_bridge.0: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
[  708.056775][ T2080] probe of port10 returned 0 after 20555 usecs
[  708.057814][ T2080] platform cxl_host_bridge.0: host supports CXL
[  708.062226][ T2080] platform cxl_host_bridge.1: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
[  708.081857][ T2080] probe of port11 returned 0 after 18696 usecs
[  708.085821][ T2080] platform cxl_host_bridge.1: host supports CXL
[  708.086496][ T2080] platform cxl_host_bridge.2: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
[  708.538268][ T2080] probe of port12 returned 0 after 450064 usecs
[  708.563248][ T2080] platform cxl_host_bridge.2: host supports CXL
[  708.563875][ T2080] platform cxl_host_bridge.3: host supports CXL (restricted)
[  708.803640][ T2080] probe of ndbus1 returned 0 after 87373 usecs
[  708.817241][ T2080] probe of nvdimm-bridge1 returned 0 after 182992 usecs
[  708.839172][ T2080] probe of cxl_acpi.0 returned 0 after 843615 usecs
[  709.026867][  T503] cxl_mock_mem cxl_mem.0: CXL MCE unsupported
[  709.240435][  T502] cxl_mock_mem cxl_mem.1: CXL MCE unsupported
[  709.263055][  T499] cxl_mock_mem cxl_mem.2: CXL MCE unsupported
[  709.317257][  T503] probe of port13 returned 0 after 57147 usecs
[  709.442524][  T498] cxl_mock_mem cxl_mem.3: CXL MCE unsupported
[  709.495789][  T499] probe of port15 returned 0 after 57022 usecs
[  709.538513][ T1514] cxl_mock_mem cxl_mem.5: CXL MCE unsupported
[  709.553021][  T503] probe of nmem3 returned 0 after 12329 usecs
[  709.555876][  T503] probe of pmem0 returned 0 after 54954 usecs
[  709.567823][  T499] probe of nmem2 returned 0 after 27577 usecs
[  709.569359][  T499] probe of pmem2 returned 0 after 64505 usecs
[  709.603845][  T497] cxl_mock_mem cxl_mem.4: CXL MCE unsupported
[  709.626487][   T12] cxl_mock_mem cxl_mem.6: CXL MCE unsupported
[  709.639194][  T502] probe of port14 returned 0 after 255539 usecs
[  709.662671][  T503] probe of region2 returned 6 after 421 usecs
[  709.664855][  T503] cxl_mock_mem cxl_mem.0: Extended linear cache calculation failed rc:-2
[  709.694975][  T503] probe of endpoint16 returned 0 after 100427 usecs
[  709.698678][  T503] probe of mem0 returned 0 after 516964 usecs
[  709.752821][   T49] cxl_mock_mem cxl_mem.7: CXL MCE unsupported
[  709.782050][  T499] probe of endpoint17 returned 0 after 102692 usecs
[  709.814422][  T499] probe of mem2 returned 0 after 539843 usecs
[  709.859496][  T497] probe of nmem4 returned 0 after 74368 usecs
[  709.860064][  T497] probe of pmem5 returned 0 after 120134 usecs
[  709.862431][  T499] probe of cxl_mem.2 returned 0 after 686512 usecs
[  709.863290][  T503] probe of cxl_mem.0 returned 0 after 892734 usecs
[  709.870924][   T30] cxl_mock_mem cxl_mem.9: CXL MCE unsupported
[  709.876631][ T2080] initcall cxl_test_init+0x0/0xc88 [cxl_test] returned 0 after 2764528 usecs
[  709.886776][  T498] probe of port18 returned 0 after 168122 usecs
[  709.900934][  T500] cxl_mock_mem cxl_mem.8: CXL MCE unsupported
[  709.946498][ T1514] probe of nmem5 returned 0 after 9462 usecs
[  709.947381][ T1514] probe of pmem4 returned 0 after 14445 usecs
[  709.970022][   T12] probe of nmem6 returned 0 after 32603 usecs
[  709.978337][  T498] probe of nmem7 returned 0 after 35875 usecs
[  709.986071][  T498] probe of pmem3 returned 0 after 46583 usecs
[  710.010717][   T12] probe of pmem6 returned 0 after 75369 usecs
[  710.014337][  T501] cxl_mock_mem cxl_rcd.10: CXL MCE unsupported
[  710.040337][  T502] probe of nmem8 returned 0 after 29862 usecs
[  710.059653][  T502] probe of pmem1 returned 0 after 88935 usecs
[  710.079573][   T12] probe of endpoint23 returned 0 after 49198 usecs
[  710.097280][   T30] probe of port21 returned 0 after 120461 usecs
[  710.097393][   T49] probe of nmem9 returned 0 after 26437 usecs
[  710.101820][   T49] probe of pmem7 returned 0 after 36944 usecs
[  710.106202][   T12] probe of mem6 returned 0 after 452293 usecs
[  710.130816][   T12] probe of cxl_mem.6 returned 0 after 669975 usecs
[  710.133959][  T500] probe of nmem10 returned 0 after 12351 usecs
[  710.170179][  T500] probe of pmem9 returned 0 after 50316 usecs
[  710.183178][  T498] probe of endpoint22 returned 0 after 160579 usecs
[  710.208790][  T498] probe of mem3 returned 0 after 760551 usecs
[  710.212463][  T501] probe of endpoint24 returned 0 after 149792 usecs
[  710.236975][  T497] probe of dax2.0 returned 0 after 118646 usecs
[  710.240952][  T497] probe of dax_region2 returned 0 after 166940 usecs
[  710.242545][  T498] probe of cxl_mem.3 returned 0 after 917362 usecs
[  710.256278][   T30] probe of nmem11 returned 0 after 45572 usecs
[  710.257079][   T30] probe of pmem8 returned 0 after 105691 usecs
[  710.259332][  T501] probe of mem10 returned 0 after 218194 usecs
[  710.265545][  T497] probe of region2 returned 0 after 225563 usecs
[  710.269232][ T1514] probe of endpoint20 returned 0 after 320269 usecs
[  710.282299][  T497] probe of endpoint19 returned 0 after 421116 usecs
[  710.283234][  T497] probe of mem5 returned 0 after 642268 usecs
[  710.304734][ T1514] probe of mem4 returned 0 after 762768 usecs
[  710.322940][   T49] probe of endpoint26 returned 0 after 117975 usecs
[  710.324643][   T49] probe of mem7 returned 0 after 558032 usecs
[  710.336434][  T501] probe of cxl_rcd.10 returned 0 after 414904 usecs
[  710.339624][  T497] probe of cxl_mem.4 returned 0 after 957535 usecs
[  710.400496][ T1514] probe of cxl_mem.5 returned 0 after 996364 usecs
[  710.450320][   T49] probe of cxl_mem.7 returned 0 after 842769 usecs
[  710.648851][  T500] probe of endpoint25 returned 0 after 477479 usecs
[  710.659334][  T500] probe of mem9 returned 0 after 690185 usecs
[  710.706030][  T500] probe of cxl_mem.8 returned 0 after 917194 usecs
[  711.162896][   T30] probe of endpoint28 returned 0 after 493899 usecs
[  711.227336][   T30] probe of mem8 returned 0 after 1278462 usecs
[  711.325568][  T502] probe of endpoint27 returned 0 after 929403 usecs
[  711.356687][  T502] probe of mem1 returned 0 after 2101415 usecs
[  711.531055][   T30] probe of cxl_mem.9 returned 0 after 1707787 usecs
[  711.554073][  T502] probe of cxl_mem.1 returned 0 after 2425696 usecs
[  724.421245][ T2077] probe of region5 returned 6 after 262 usecs
1/1 ndctl:cxl / cxl-region-sysfs.sh        FAIL            18.22s   exit status 1
>>> TEST_PATH=/root/ndctl/build/test ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 LD_LIBRARY_PATH=/root/ndctl/build/daxctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/ndctl/lib DAXCTL=/root/ndctl/build/daxctl/daxctl DATA_PATH=/root/ndctl/test NDCTL=/root/ndctl/build/ndctl/ndctl MESON_TEST_ITERATION=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MALLOC_PERTURB_=123 /bin/bash /root/ndctl/test/cxl-region-sysfs.sh

The kernel (the cxl_test kernel module) is built off of cxl/next which
has Jonathan's fix to the cxl_test seen on arm64.
Could the experts take a look at this issue?

Thanks,
Itaru.

> 
> Jonathan Cameron (4):
>   hw/cxl-host: Add an index field to CXLFixedMemoryWindow
>   hw/cxl: Make the CXL fixed memory windows devices.
>   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      |   5 +-
>  include/hw/cxl/cxl_host.h |   5 +-
>  hw/acpi/cxl.c             |  76 +++++++++--------
>  hw/arm/virt-acpi-build.c  |  34 ++++++++
>  hw/arm/virt.c             |  29 +++++++
>  hw/cxl/cxl-host-stubs.c   |   7 +-
>  hw/cxl/cxl-host.c         | 170 +++++++++++++++++++++++++++++++-------
>  hw/i386/pc.c              |  50 +++++------
>  tests/qtest/cxl-test.c    |  59 ++++++++++---
>  tests/qtest/meson.build   |   1 +
>  11 files changed, 330 insertions(+), 110 deletions(-)
> 
> -- 
> 2.48.1
> 

Re: [PATCH v15 4/4] qtest/cxl: Add aarch64 virt test for CXL

[Peter Maydell]

On Thu, 12 Jun 2025 at 14:45, 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>
>
> ---
> v15: Dropped tags due to changes in patches 2 and 3.
> ---
>  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);

We prefer g_mkdtemp() or g_dir_make_temp() over raw mkdtemp(),
I think. Other tests in this file use g_dir_make_tmp().

Also you aren't checking whether it failed.

> +
> +    g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
> +                    tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
> +                    tmpfs, tmpfs);
> +
> +    qtest_start(cmdline->str);

We never change the GString and only use its C representation,
so I think it's simpler to use
  g_autofree char *cmdline = NULL;
  ...
  cmdline = g_strdup_printf(...);

But I see all the other tests in this file are written this
way, so I'm ok with staying consistent to that here.

-- PMM

Re: [PATCH v15 2/4] hw/cxl: Make the CXL fixed memory windows devices.

[Peter Maydell]

On Thu, 12 Jun 2025 at 14:44, Jonathan Cameron
<Jonathan.Cameron@huawei.com> 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.


> +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;
> +}

Do these things have any state that needs migrating or resetting?
If they do, they need a reset function and a vmstate. If not,
it's helpful to have a comment explaining that the device
has no state to be reset or migrated, so future readers of
the code know this wasn't just accidentally forgotten.

thanks
-- PMM

Re: [PATCH v15 3/4] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl

[Peter Maydell]

On Thu, 12 Jun 2025 at 14:45, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> 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>
>
> ---
> v15: No changes.
> ---
>  include/hw/arm/virt.h    |  4 ++++
>  hw/arm/virt-acpi-build.c | 34 ++++++++++++++++++++++++++++++++++
>  hw/arm/virt.c            | 29 +++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+)

Can we have some user-facing documentation, please?
(docs/system/arm/virt.rst -- can just be a brief mention
and link to docs/system/devices/cxl.rst if you want to put the
examples of aarch64 use in there.)

> @@ -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 */

This is going to shuffle the memory map around, even if CXL
isn't enabled, which will break migration compatibility.
You need to do something to ensure that the CXL region isn't
included in the calculations of the base addresses for these
regions if CXL isn't enabled.

>      [VIRT_HIGH_PCIE_ECAM] =     { 0x0, 256 * MiB },
>      /* Second PCIe window */
>      [VIRT_HIGH_PCIE_MMIO] =     { 0x0, DEFAULT_HIGH_PCIE_MMIO_SIZE },

If you're OK with having the CXL host region at the end of the
list then that's a simpler way to avoid its presence disturbing
the layout of the existing regions, but you might not like it
being at such a high physaddr.

> @@ -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;

This looks odd -- why are we reaching directly into the cxl_devices_state
to fish out a MemoryRegion and init it?

> +    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));

Isn't this stomping over the HIGH_PCIE memory window (or
whatever else happens to be at the top end of memory) ?

Also taking highest_gpa and then rounding it up looks suspicious:
if it's the highest GPA then anything larger than that is off
the end of the physical address space.

Plus cxl_fmws_set_memmap() names its arguments base, max_addr:
"highest gpa, rounded up" doesn't sound like a base address.

(Looking at our current code that sets and adjusts highest_gpa,
it looks a bit weird: maybe we're setting it to values that aren't
what the variable name claims it's doing, and that's why this
code happens to work ?)

>  }
>
>  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);
>  }

cxl defaults to disabled, right? (i.e. we don't need the
machine-version specific stuff to keep it from being enabled
on old versioned machine types).

thanks
-- PMM

Re: [PATCH v15 2/4] hw/cxl: Make the CXL fixed memory windows devices.

[Jonathan Cameron via]

On Fri, 13 Jun 2025 13:33:51 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 12 Jun 2025 at 14:44, Jonathan Cameron
> <Jonathan.Cameron@huawei.com> 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.  
> 
> 
> > +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;
> > +}  
> 
> Do these things have any state that needs migrating or resetting?
> If they do, they need a reset function and a vmstate. If not,
> it's helpful to have a comment explaining that the device
> has no state to be reset or migrated, so future readers of
> the code know this wasn't just accidentally forgotten.

Hi Peter,

For these specific devices (the fixed memory windows) there isn't
any state as they are representing fixed configuration of the system.
The state is all in the host bridges and beyond. I'll add
a comment as you suggest.

Currently CXL emulation is completely broken wrt to migration and
there are some known issues for reset as well. Both are on the list
of things to fix. Migration is less important as the only current use
for this stuff is running software stack test cases and for that
migration isn't currently of interest - that will change for some
of the virtualization related work that is just getting started.
The reset thing needs more work for devices as we've tripped over
a few corner cases triggered by people rebooting the guest and device
state not being fully cleared.  On top of that we have a complex
nest of device reset types to cover at some point as some registers
are sticky over some types of reset.

Jonathan



> 
> thanks
> -- PMM

Re: [PATCH v15 3/4] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl

[Jonathan Cameron via]

On Fri, 13 Jun 2025 13:57:39 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 12 Jun 2025 at 14:45, Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > 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>
> >
> > ---
> > v15: No changes.
> > ---
> >  include/hw/arm/virt.h    |  4 ++++
> >  hw/arm/virt-acpi-build.c | 34 ++++++++++++++++++++++++++++++++++
> >  hw/arm/virt.c            | 29 +++++++++++++++++++++++++++++
> >  3 files changed, 67 insertions(+)  
> 
Hi Peter,

Thanks for reviewing.

> Can we have some user-facing documentation, please?
> (docs/system/arm/virt.rst -- can just be a brief mention
> and link to docs/system/devices/cxl.rst if you want to put the
> examples of aarch64 use in there.)

Given the examples should look exactly like those for x86/pc, do we need
extra examples in cxl.rst? I guess I can add one simple arm/virt example
in a follow up patch without bloating that file too badly..

Is the following sufficient for the board specific docs?

diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 6a719b9586..10cbffc8a7 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -31,6 +31,7 @@ Supported devices
 The virt board supports:

 - PCI/PCIe devices
+- CXL Fixed memory windows, root bridges and devices.
 - Flash memory
 - Either one or two PL011 UARTs for the NonSecure World
 - An RTC
@@ -189,6 +190,14 @@ ras
 acpi
   Set ``on``/``off``/``auto`` to enable/disable ACPI.

+cxl
+  Set  ``on``/``off`` to enable/disable CXL. More details in
+  :doc:`../devices/cxl`. The default is off.
+
+cxl-fmw
+  Array of CXL fixed memory windows describing fixed address routing to
+  target CXL host bridges. See :doc:`../devices/cxl`.
+
 dtb-randomness
   Set ``on``/``off`` to pass random seeds via the guest DTB
   rng-seed and kaslr-seed nodes (in both "/chosen" and

> 
> > @@ -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 */  
> 
> This is going to shuffle the memory map around, even if CXL
> isn't enabled, which will break migration compatibility.
> You need to do something to ensure that the CXL region isn't
> included in the calculations of the base addresses for these
> regions if CXL isn't enabled.
> 

It doesn't move any existing stuff because these are naturally aligned
regions so this is in a gap before the PCIE ECAM region.

> >      [VIRT_HIGH_PCIE_ECAM] =     { 0x0, 256 * MiB },
> >      /* Second PCIe window */
> >      [VIRT_HIGH_PCIE_MMIO] =     { 0x0, DEFAULT_HIGH_PCIE_MMIO_SIZE },  
> 
> If you're OK with having the CXL host region at the end of the
> list then that's a simpler way to avoid its presence disturbing
> the layout of the existing regions, but you might not like it
> being at such a high physaddr.

From what I recall a higher address isn't a problem I just wanted to not waste any
PA space at all so used the gap.

Chunk of /proc/iomem with a random test case (in first case with the cxl bits
removed as obvious that doesn't start until this patch is in place).
Need more than 123 cpus to make the second gicv3 redist appear
(I've no idea why that number I just printed the threshold where
it was calculated to find out what I needed to wait for boot on).

before this patch.

13ffe0000-13fffffff : System RAM
  13ffe0000-13ffe1fff : reserved
  13ffe2000-13ffe2fff : reserved
  13ffe3000-13fffffff : reserved
4000000000-4003ffffff : GICR
4010000000-401fffffff : PCI ECAM
8000000000-ffffffffff : PCI Bus 0000:00
  8000000000-80001fffff : PCI Bus 0000:01
  8000200000-8000203fff : 0000:00:02.0
    8000200000-8000203fff : virtio-pci-modern
  8000204000-8000207fff : 0000:00:03.0
    8000204000-8000207fff : virtio-pci-modern

after:

13ffe0000-13fffffff : System RAM
  13ffe0000-13ffe2fff : reserved
  13ffe3000-13fff3fff : reserved
  13fff4000-13fffffff : reserved
4000000000-4003ffffff : GICR
4004001128-40040011b7 : port1
4010000000-4010bfffff : PCI ECAM
4010c00000-4010efffff : PCI ECAM
8000000000-80000fffff : PCI Bus 0000:00
  8000000000-8000003fff : 0000:00:02.0
    8000000000-8000003fff : virtio-pci-modern
  8000004000-8000007fff : 0000:00:03.0
    8000004000-8000007fff : virtio-pci-modern

The extra ECAM is the PXB stuff kicking in and stealing part of the ECAM
region of the main host bridge.

The CXL bit we care about here is port1. Despite the name which is
an artifact of how linux represents the topology, that is
the registers for the CXL PXB root bridge.


> 
> > @@ -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;  
> 
> This looks odd -- why are we reaching directly into the cxl_devices_state
> to fish out a MemoryRegion and init it?

cxl_devices_state state is really just grouping the CXL host stuff that
every cxl host needs so this perhaps looks like more than it actually is.
Maybe that needs a rename. It does less than used to after patch 2 took
some of the Fixed memory stuff out of there.

typedef struct CXLState {
    bool is_enabled;
    MemoryRegion host_mr;
    unsigned int next_mr_idx;
    CXLFixedMemoryWindowOptionsList *cfmw_list;
} CXLState;

It has two purposes.  The cfmw_list and is_enabled are so that
we can use cxl_host_init() to unify setting up the CXL specific machine
properties.

host_mr and next_mr_idx are for a very simple fixed chunk address space
allocator for the PXB-CXL base registers. This is a little bit nasty but
it allows the PXBs to be initialized with -device and yet have mmio
address ranges. Bit similar to the runtime instantiation of SMMUs
problem Shameer was running into, but a simpler one.  I couldn't
find a way to use sysbus or similar here because we also need this
PXB device to exist on the main PCI bus like any other PCI expander bridge
- it just needs these extra registers in the main memory map that are then
pointed to by ACPI table entries. These are what is known as RCRB in PCI
terms (Root complex register base).

See implementation of pxb_cxl_hook_up_registers() call in
virt_machine_done() that stitches this together once we know what
pxb-cxl devices are present and maps sub regions into this container.

I'd be happy to have suggestions on how to do this better as I've never
been fond of this bit.

One option might be to just make this more explicit by putting host_mr and
next_mr_idx directly in the virt machine state and passing them
as separate parameters to pxb_cxl_hook_up_registers(). I'm not sure
that gains much though.  Another path would be to wrap the region init
and add subregion up in a helper function called from here and i386/pc.c
That would mean the name at least was explicitly shared. Not sure it would
bring much other benefit.


> 
> > +    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));  
> 
> Isn't this stomping over the HIGH_PCIE memory window (or
> whatever else happens to be at the top end of memory) ?

I'll admit the way that these address maps are calculated has caused
me several headaches.  The intention was to go above everything so as to
avoid any changes to the existing map.

> 
> Also taking highest_gpa and then rounding it up looks suspicious:
> if it's the highest GPA then anything larger than that is off
> the end of the physical address space.
> 
> Plus cxl_fmws_set_memmap() names its arguments base, max_addr:
> "highest gpa, rounded up" doesn't sound like a base address.
> 
> (Looking at our current code that sets and adjusts highest_gpa,
> it looks a bit weird: maybe we're setting it to values that aren't
> what the variable name claims it's doing, and that's why this
> code happens to work ?)

I think highest_gpa is a running value of how high we have allocated
to something, not a maximum that is supported.  E.g. what is going on
in virt_set_high_memmap()

I guess I should update vms->highest_gpa.  I think nothing uses it after this
but they might in future.

+    vms->highest_gpa = cxl_fmws_set_memmap(ROUND_UP(vms->highest_gpa + 1, 256 * MiB),
+                                           BIT_ULL(pa_bits)) - 1;

should update it correctly. In i386/pc.c we already updated the memory map top address
tracking so that function already returns the start of the next region (hence the - 1
is needed - that is similar to the call for virt_set_high_memmap() a few lines up.

I think that will also result in more elegant failure on CPUs with limited
address space than we had before as that is checked in virt_cpu_post_init()

Using an a55 for instance fails (Fujitsu folk ran into this a while back)
as we need more than 40 bits.

Just for reference here's the relevant bit of /proc/iomem
for a test setup intended to poke some of these corners.

8000000000-80000fffff : PCI Bus 0000:00
  8000000000-8000003fff : 0000:00:02.0
    8000000000-8000003fff : virtio-pci-modern
  8000004000-8000007fff : 0000:00:03.0
    8000004000-8000007fff : virtio-pci-modern
8000100000-800011ffff : PCI Bus 0000:0c
  8000100000-800010ffff : 0000:0c:00.0
    8000101080-80001010d7 : mem0
  8000110000-800011ffff : 0000:0c:01.0
    8000111080-80001110d7 : mem1
8000120000-ffffffffff : PCI Bus 0000:00
  8000200000-80003fffff : PCI Bus 0000:01
10000000000-101ffffffff : CXL Window 0
  10000000000-1000fffffff : region0
    10000000000-1000fffffff : dax0.0
      10000000000-1000fffffff : System RAM (kmem)

So the windows sits above the PCI region.

> 
> >  }
> >
> >  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);
> >  }  
> 
> cxl defaults to disabled, right? (i.e. we don't need the
> machine-version specific stuff to keep it from being enabled
> on old versioned machine types).

It is indeed defaulting to disabled.

J

> 
> thanks
> -- PMM

Re: [PATCH v15 0/4] arm/virt: CXL support via pxb_cxl

[Jonathan Cameron via]

On Fri, 13 Jun 2025 14:07:32 +0900
Itaru Kitayama <itaru.kitayama@linux.dev> wrote:

> On Thu, Jun 12, 2025 at 02:43:34PM +0100, Jonathan Cameron wrote:
> > v15:
> >   - Split the address map calculations and mmio setup into separate
> >     functions in patch 2, allowing v14 patch 3 to be dropped as not
> >     x86 and arm make the same calls.  Note I felt this was a sufficient
> >     change to trigger dropping tags. (Zhijian Li)
> >   - A few other minor tweaks.
> >   - TLB issue mentioned in v14 now fixed upstream so dropped reference
> >     in this cover letter.
> > 
> > Thanks to Itaru Kitayama and Zhijian Li for testing + reviews.
> > 
> > Updated 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>.  Will update shortly with this
> > series.
> > 
> > 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!)
> > 
> > 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.
> > 
> > Also, we are kicking off some work on DCD virtualization, particularly to
> > support inter-host shared memory being presented up into a VM. That
> > will need upstream support on arm64 as it is built on top of the existing
> > CXL emulation to avoid the need for a separate guest software stack.
> > 
> > Note this is TCG only - it is possible to support limited use with KVM but
> > that needs additional patches not yet ready for upstream.  The challenge
> > is interleave - and the solution is don't interleave if you want to run
> > with KVM.  
> 
> One of the ndctl:cxl tests fails (other tests ran ok):

Whilst interesting and needing debugging I'd just like to point out that those
tests are not running on the qemu CXL emulation at all, but on cxl test which
is mocking stuff in the kernel.   I know you know that but it might not
be obvious to anyone seeing a bug report on this thread!

Jonathan


> 
> # meson test cxl-region-sysfs.sh
> ninja: Entering directory `/root/ndctl/build'
> [1/55] Generating version.h with a custom command
> [  706.564783][ T2080] calling  cxl_port_init+0x0/0xfe0 [cxl_port] @ 2080
> [  706.566861][ T2080] initcall cxl_port_init+0x0/0xfe0 [cxl_port] returned 0 after 1735 usecs
> [  706.586457][ T2080] calling  cxl_acpi_init+0x0/0xfe0 [cxl_acpi] @ 2080
> [  706.625690][ T2080] probe of port1 returned 0 after 25381 usecs
> [  706.626634][ T2080]  pci0000:bf: host supports CXL
> [  706.653573][ T2080] probe of port2 returned 0 after 25631 usecs
> [  706.655164][ T2080]  pci0000:35: host supports CXL
> [  706.662409][ T2080] probe of ACPI0017:00 returned 0 after 74464 usecs
> [  706.663150][ T2080] initcall cxl_acpi_init+0x0/0xfe0 [cxl_acpi] returned 0 after 76306 usecs
> [  706.690482][ T2080] calling  cxl_pmem_init+0x0/0xfd0 [cxl_pmem] @ 2080
> [  706.695324][ T2080] probe of ndbus0 returned 0 after 1496 usecs
> [  706.699217][ T2080] probe of nvdimm-bridge0 returned 0 after 6705 usecs
> [  706.702372][ T2080] initcall cxl_pmem_init+0x0/0xfd0 [cxl_pmem] returned 0 after 11576 usecs
> [  706.717668][ T2080] calling  cxl_mem_driver_init+0x0/0xfe0 [cxl_mem] @ 2080
> [  706.758561][ T2080] probe of port3 returned 0 after 34188 usecs
> [  706.767080][ T2080] cxl_nvdimm pmem11: GPF: could not set dirty shutdown state
> [  706.779392][ T2080] probe of nmem0 returned 0 after 1083 usecs
> [  706.782181][ T2080] probe of pmem11 returned 0 after 15516 usecs
> [  706.826941][ T2080] probe of endpoint4 returned 0 after 42630 usecs
> [  706.827987][ T2080] probe of mem11 returned 0 after 108475 usecs
> [  706.878052][ T2080] probe of port5 returned 0 after 41354 usecs
> [  706.938260][ T2080] probe of endpoint6 returned 0 after 46831 usecs
> [  706.939223][ T2080] probe of mem12 returned 0 after 105104 usecs
> [  706.994611][ T2080] probe of endpoint7 returned 0 after 49337 usecs
> [  706.995790][ T2080] probe of mem13 returned 0 after 53632 usecs
> [  707.004334][ T2080] cxl_nvdimm pmem14: GPF: could not set dirty shutdown state
> [  707.017782][ T2080] probe of nmem1 returned 0 after 1115 usecs
> [  707.019324][ T2080] probe of pmem14 returned 0 after 14920 usecs
> [  707.072148][ T2080] probe of endpoint8 returned 0 after 50887 usecs
> [  707.073367][ T2080] probe of mem14 returned 0 after 71279 usecs
> [  707.079062][ T2080] initcall cxl_mem_driver_init+0x0/0xfe0 [cxl_mem] returned 0 after 361073 usecs
> [  707.111533][ T2080] calling  cxl_test_init+0x0/0xc88 [cxl_test] @ 2080
> [  708.001403][ T2080] platform cxl_host_bridge.0: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
> [  708.002032][ T2080] platform cxl_host_bridge.1: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
> [  708.010988][ T2080] platform cxl_host_bridge.2: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
> [  708.011963][ T2080] platform cxl_host_bridge.3: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
> [  708.034604][ T2080] platform cxl_host_bridge.0: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
> [  708.056775][ T2080] probe of port10 returned 0 after 20555 usecs
> [  708.057814][ T2080] platform cxl_host_bridge.0: host supports CXL
> [  708.062226][ T2080] platform cxl_host_bridge.1: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
> [  708.081857][ T2080] probe of port11 returned 0 after 18696 usecs
> [  708.085821][ T2080] platform cxl_host_bridge.1: host supports CXL
> [  708.086496][ T2080] platform cxl_host_bridge.2: Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.
> [  708.538268][ T2080] probe of port12 returned 0 after 450064 usecs
> [  708.563248][ T2080] platform cxl_host_bridge.2: host supports CXL
> [  708.563875][ T2080] platform cxl_host_bridge.3: host supports CXL (restricted)
> [  708.803640][ T2080] probe of ndbus1 returned 0 after 87373 usecs
> [  708.817241][ T2080] probe of nvdimm-bridge1 returned 0 after 182992 usecs
> [  708.839172][ T2080] probe of cxl_acpi.0 returned 0 after 843615 usecs
> [  709.026867][  T503] cxl_mock_mem cxl_mem.0: CXL MCE unsupported
> [  709.240435][  T502] cxl_mock_mem cxl_mem.1: CXL MCE unsupported
> [  709.263055][  T499] cxl_mock_mem cxl_mem.2: CXL MCE unsupported
> [  709.317257][  T503] probe of port13 returned 0 after 57147 usecs
> [  709.442524][  T498] cxl_mock_mem cxl_mem.3: CXL MCE unsupported
> [  709.495789][  T499] probe of port15 returned 0 after 57022 usecs
> [  709.538513][ T1514] cxl_mock_mem cxl_mem.5: CXL MCE unsupported
> [  709.553021][  T503] probe of nmem3 returned 0 after 12329 usecs
> [  709.555876][  T503] probe of pmem0 returned 0 after 54954 usecs
> [  709.567823][  T499] probe of nmem2 returned 0 after 27577 usecs
> [  709.569359][  T499] probe of pmem2 returned 0 after 64505 usecs
> [  709.603845][  T497] cxl_mock_mem cxl_mem.4: CXL MCE unsupported
> [  709.626487][   T12] cxl_mock_mem cxl_mem.6: CXL MCE unsupported
> [  709.639194][  T502] probe of port14 returned 0 after 255539 usecs
> [  709.662671][  T503] probe of region2 returned 6 after 421 usecs
> [  709.664855][  T503] cxl_mock_mem cxl_mem.0: Extended linear cache calculation failed rc:-2
> [  709.694975][  T503] probe of endpoint16 returned 0 after 100427 usecs
> [  709.698678][  T503] probe of mem0 returned 0 after 516964 usecs
> [  709.752821][   T49] cxl_mock_mem cxl_mem.7: CXL MCE unsupported
> [  709.782050][  T499] probe of endpoint17 returned 0 after 102692 usecs
> [  709.814422][  T499] probe of mem2 returned 0 after 539843 usecs
> [  709.859496][  T497] probe of nmem4 returned 0 after 74368 usecs
> [  709.860064][  T497] probe of pmem5 returned 0 after 120134 usecs
> [  709.862431][  T499] probe of cxl_mem.2 returned 0 after 686512 usecs
> [  709.863290][  T503] probe of cxl_mem.0 returned 0 after 892734 usecs
> [  709.870924][   T30] cxl_mock_mem cxl_mem.9: CXL MCE unsupported
> [  709.876631][ T2080] initcall cxl_test_init+0x0/0xc88 [cxl_test] returned 0 after 2764528 usecs
> [  709.886776][  T498] probe of port18 returned 0 after 168122 usecs
> [  709.900934][  T500] cxl_mock_mem cxl_mem.8: CXL MCE unsupported
> [  709.946498][ T1514] probe of nmem5 returned 0 after 9462 usecs
> [  709.947381][ T1514] probe of pmem4 returned 0 after 14445 usecs
> [  709.970022][   T12] probe of nmem6 returned 0 after 32603 usecs
> [  709.978337][  T498] probe of nmem7 returned 0 after 35875 usecs
> [  709.986071][  T498] probe of pmem3 returned 0 after 46583 usecs
> [  710.010717][   T12] probe of pmem6 returned 0 after 75369 usecs
> [  710.014337][  T501] cxl_mock_mem cxl_rcd.10: CXL MCE unsupported
> [  710.040337][  T502] probe of nmem8 returned 0 after 29862 usecs
> [  710.059653][  T502] probe of pmem1 returned 0 after 88935 usecs
> [  710.079573][   T12] probe of endpoint23 returned 0 after 49198 usecs
> [  710.097280][   T30] probe of port21 returned 0 after 120461 usecs
> [  710.097393][   T49] probe of nmem9 returned 0 after 26437 usecs
> [  710.101820][   T49] probe of pmem7 returned 0 after 36944 usecs
> [  710.106202][   T12] probe of mem6 returned 0 after 452293 usecs
> [  710.130816][   T12] probe of cxl_mem.6 returned 0 after 669975 usecs
> [  710.133959][  T500] probe of nmem10 returned 0 after 12351 usecs
> [  710.170179][  T500] probe of pmem9 returned 0 after 50316 usecs
> [  710.183178][  T498] probe of endpoint22 returned 0 after 160579 usecs
> [  710.208790][  T498] probe of mem3 returned 0 after 760551 usecs
> [  710.212463][  T501] probe of endpoint24 returned 0 after 149792 usecs
> [  710.236975][  T497] probe of dax2.0 returned 0 after 118646 usecs
> [  710.240952][  T497] probe of dax_region2 returned 0 after 166940 usecs
> [  710.242545][  T498] probe of cxl_mem.3 returned 0 after 917362 usecs
> [  710.256278][   T30] probe of nmem11 returned 0 after 45572 usecs
> [  710.257079][   T30] probe of pmem8 returned 0 after 105691 usecs
> [  710.259332][  T501] probe of mem10 returned 0 after 218194 usecs
> [  710.265545][  T497] probe of region2 returned 0 after 225563 usecs
> [  710.269232][ T1514] probe of endpoint20 returned 0 after 320269 usecs
> [  710.282299][  T497] probe of endpoint19 returned 0 after 421116 usecs
> [  710.283234][  T497] probe of mem5 returned 0 after 642268 usecs
> [  710.304734][ T1514] probe of mem4 returned 0 after 762768 usecs
> [  710.322940][   T49] probe of endpoint26 returned 0 after 117975 usecs
> [  710.324643][   T49] probe of mem7 returned 0 after 558032 usecs
> [  710.336434][  T501] probe of cxl_rcd.10 returned 0 after 414904 usecs
> [  710.339624][  T497] probe of cxl_mem.4 returned 0 after 957535 usecs
> [  710.400496][ T1514] probe of cxl_mem.5 returned 0 after 996364 usecs
> [  710.450320][   T49] probe of cxl_mem.7 returned 0 after 842769 usecs
> [  710.648851][  T500] probe of endpoint25 returned 0 after 477479 usecs
> [  710.659334][  T500] probe of mem9 returned 0 after 690185 usecs
> [  710.706030][  T500] probe of cxl_mem.8 returned 0 after 917194 usecs
> [  711.162896][   T30] probe of endpoint28 returned 0 after 493899 usecs
> [  711.227336][   T30] probe of mem8 returned 0 after 1278462 usecs
> [  711.325568][  T502] probe of endpoint27 returned 0 after 929403 usecs
> [  711.356687][  T502] probe of mem1 returned 0 after 2101415 usecs
> [  711.531055][   T30] probe of cxl_mem.9 returned 0 after 1707787 usecs
> [  711.554073][  T502] probe of cxl_mem.1 returned 0 after 2425696 usecs
> [  724.421245][ T2077] probe of region5 returned 6 after 262 usecs
> 1/1 ndctl:cxl / cxl-region-sysfs.sh        FAIL            18.22s   exit status 1
> >>> TEST_PATH=/root/ndctl/build/test ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 LD_LIBRARY_PATH=/root/ndctl/build/daxctl/lib:/root/ndctl/build/cxl/lib:/root/ndctl/build/ndctl/lib DAXCTL=/root/ndctl/build/daxctl/daxctl DATA_PATH=/root/ndctl/test NDCTL=/root/ndctl/build/ndctl/ndctl MESON_TEST_ITERATION=1 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 MALLOC_PERTURB_=123 /bin/bash /root/ndctl/test/cxl-region-sysfs.sh  
> 
> The kernel (the cxl_test kernel module) is built off of cxl/next which
> has Jonathan's fix to the cxl_test seen on arm64.
> Could the experts take a look at this issue?
> 
> Thanks,
> Itaru.
> 
> > 
> > Jonathan Cameron (4):
> >   hw/cxl-host: Add an index field to CXLFixedMemoryWindow
> >   hw/cxl: Make the CXL fixed memory windows devices.
> >   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      |   5 +-
> >  include/hw/cxl/cxl_host.h |   5 +-
> >  hw/acpi/cxl.c             |  76 +++++++++--------
> >  hw/arm/virt-acpi-build.c  |  34 ++++++++
> >  hw/arm/virt.c             |  29 +++++++
> >  hw/cxl/cxl-host-stubs.c   |   7 +-
> >  hw/cxl/cxl-host.c         | 170 +++++++++++++++++++++++++++++++-------
> >  hw/i386/pc.c              |  50 +++++------
> >  tests/qtest/cxl-test.c    |  59 ++++++++++---
> >  tests/qtest/meson.build   |   1 +
> >  11 files changed, 330 insertions(+), 110 deletions(-)
> > 
> > -- 
> > 2.48.1
> >   

Re: [PATCH v15 3/4] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl

[Peter Maydell]

On Fri, 13 Jun 2025 at 16:20, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Fri, 13 Jun 2025 13:57:39 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
> > On Thu, 12 Jun 2025 at 14:45, Jonathan Cameron
> > <Jonathan.Cameron@huawei.com> wrote:
> > >
> > > 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>
> > >
> > > ---
> > > v15: No changes.
> > > ---
> > >  include/hw/arm/virt.h    |  4 ++++
> > >  hw/arm/virt-acpi-build.c | 34 ++++++++++++++++++++++++++++++++++
> > >  hw/arm/virt.c            | 29 +++++++++++++++++++++++++++++
> > >  3 files changed, 67 insertions(+)
> >
> Hi Peter,
>
> Thanks for reviewing.
>
> > Can we have some user-facing documentation, please?
> > (docs/system/arm/virt.rst -- can just be a brief mention
> > and link to docs/system/devices/cxl.rst if you want to put the
> > examples of aarch64 use in there.)
>
> Given the examples should look exactly like those for x86/pc, do we need
> extra examples in cxl.rst? I guess I can add one simple arm/virt example
> in a follow up patch without bloating that file too badly..

That's fine too -- if the answer is "all these command lines work
for aarch64 too", then you can just say that in cxl.rst.

> Is the following sufficient for the board specific docs?
>
> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> index 6a719b9586..10cbffc8a7 100644
> --- a/docs/system/arm/virt.rst
> +++ b/docs/system/arm/virt.rst
> @@ -31,6 +31,7 @@ Supported devices
>  The virt board supports:
>
>  - PCI/PCIe devices
> +- CXL Fixed memory windows, root bridges and devices.
>  - Flash memory
>  - Either one or two PL011 UARTs for the NonSecure World
>  - An RTC
> @@ -189,6 +190,14 @@ ras
>  acpi
>    Set ``on``/``off``/``auto`` to enable/disable ACPI.
>
> +cxl
> +  Set  ``on``/``off`` to enable/disable CXL. More details in
> +  :doc:`../devices/cxl`. The default is off.
> +
> +cxl-fmw
> +  Array of CXL fixed memory windows describing fixed address routing to
> +  target CXL host bridges. See :doc:`../devices/cxl`.
> +
>  dtb-randomness
>    Set ``on``/``off`` to pass random seeds via the guest DTB
>    rng-seed and kaslr-seed nodes (in both "/chosen" and

Looks OK.

> >
> > > @@ -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 */
> >
> > This is going to shuffle the memory map around, even if CXL
> > isn't enabled, which will break migration compatibility.
> > You need to do something to ensure that the CXL region isn't
> > included in the calculations of the base addresses for these
> > regions if CXL isn't enabled.
> >
>
> It doesn't move any existing stuff because these are naturally aligned
> regions so this is in a gap before the PCIE ECAM region.

Is that true even when we have the maximum number of CPUs and
so the max number of redistributors in that VIRT_HIGH_GIC_REDIST2
region ?

> > >      [VIRT_HIGH_PCIE_ECAM] =     { 0x0, 256 * MiB },
> > >      /* Second PCIe window */
> > >      [VIRT_HIGH_PCIE_MMIO] =     { 0x0, DEFAULT_HIGH_PCIE_MMIO_SIZE },
> >
> > If you're OK with having the CXL host region at the end of the
> > list then that's a simpler way to avoid its presence disturbing
> > the layout of the existing regions, but you might not like it
> > being at such a high physaddr.
>
> From what I recall a higher address isn't a problem I just wanted to not waste any
> PA space at all so used the gap.
>
> Chunk of /proc/iomem with a random test case (in first case with the cxl bits
> removed as obvious that doesn't start until this patch is in place).
> Need more than 123 cpus to make the second gicv3 redist appear
> (I've no idea why that number I just printed the threshold where
> it was calculated to find out what I needed to wait for boot on).

It's 123 because that's the most redistributors we can fit into
the lower redistributor region. (We didn't really allow enough
space in the lower memory map, which is why we need this awkward
split setup.

(I have to run now, will look at the rest of the email next week.)

-- PMM

Re: [PATCH v15 2/4] hw/cxl: Make the CXL fixed memory windows devices.

[Peter Maydell]

On Fri, 13 Jun 2025 at 14:10, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
> For these specific devices (the fixed memory windows) there isn't
> any state as they are representing fixed configuration of the system.
> The state is all in the host bridges and beyond. I'll add
> a comment as you suggest.
>
> Currently CXL emulation is completely broken wrt to migration and
> there are some known issues for reset as well. Both are on the list
> of things to fix. Migration is less important as the only current use
> for this stuff is running software stack test cases and for that
> migration isn't currently of interest - that will change for some
> of the virtualization related work that is just getting started.

That's OK as long as something somewhere is registering a
migration-blocker so there's a useful error message if the
user ever tries it.

-- PMM

Re: [PATCH v15 4/4] qtest/cxl: Add aarch64 virt test for CXL

[Jonathan Cameron via]

On Fri, 13 Jun 2025 13:32:03 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 12 Jun 2025 at 14:45, 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>
> >
> > ---
> > v15: Dropped tags due to changes in patches 2 and 3.
> > ---
> >  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);  
> 
> We prefer g_mkdtemp() or g_dir_make_temp() over raw mkdtemp(),
> I think. Other tests in this file use g_dir_make_tmp().

Ah. I'd failed to update this when various people cleaned up the other
tests.  Your comments reflect that earlier cleanup so I'll update
this to match the other tests.

> 
> Also you aren't checking whether it failed.
> 
> > +
> > +    g_string_printf(cmdline, QEMU_VIRT_2PXB_CMD QEMU_4RP QEMU_4T3D,
> > +                    tmpfs, tmpfs, tmpfs, tmpfs, tmpfs, tmpfs,
> > +                    tmpfs, tmpfs);
> > +
> > +    qtest_start(cmdline->str);  
> 
> We never change the GString and only use its C representation,
> so I think it's simpler to use
>   g_autofree char *cmdline = NULL;
>   ...
>   cmdline = g_strdup_printf(...);
> 
> But I see all the other tests in this file are written this
> way, so I'm ok with staying consistent to that here.

I'll stick to matching the others.

> 
> -- PMM

Re: [PATCH v15 2/4] hw/cxl: Make the CXL fixed memory windows devices.

[Jonathan Cameron via]

On Fri, 13 Jun 2025 17:08:28 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Fri, 13 Jun 2025 at 14:10, Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> > For these specific devices (the fixed memory windows) there isn't
> > any state as they are representing fixed configuration of the system.
> > The state is all in the host bridges and beyond. I'll add
> > a comment as you suggest.
> >
> > Currently CXL emulation is completely broken wrt to migration and
> > there are some known issues for reset as well. Both are on the list
> > of things to fix. Migration is less important as the only current use
> > for this stuff is running software stack test cases and for that
> > migration isn't currently of interest - that will change for some
> > of the virtualization related work that is just getting started.  
> 
> That's OK as long as something somewhere is registering a
> migration-blocker so there's a useful error message if the
> user ever tries it.
:(

Michael asked for us to fix that a while back and I forgot.

Will sort that out shortly.

J
> 
> -- PMM

Re: [PATCH v15 3/4] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl

[Jonathan Cameron via]

On Fri, 13 Jun 2025 17:07:24 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Fri, 13 Jun 2025 at 16:20, Jonathan Cameron
> <Jonathan.Cameron@huawei.com> wrote:
> >
> > On Fri, 13 Jun 2025 13:57:39 +0100
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >  
> > > On Thu, 12 Jun 2025 at 14:45, Jonathan Cameron
> > > <Jonathan.Cameron@huawei.com> wrote:  
> > > >
> > > > 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>
> > > >
> > > > ---
> > > > v15: No changes.
> > > > ---
> > > >  include/hw/arm/virt.h    |  4 ++++
> > > >  hw/arm/virt-acpi-build.c | 34 ++++++++++++++++++++++++++++++++++
> > > >  hw/arm/virt.c            | 29 +++++++++++++++++++++++++++++
> > > >  3 files changed, 67 insertions(+)  
> > >  
> > Hi Peter,
> >
> > Thanks for reviewing.
> >  
> > > Can we have some user-facing documentation, please?
> > > (docs/system/arm/virt.rst -- can just be a brief mention
> > > and link to docs/system/devices/cxl.rst if you want to put the
> > > examples of aarch64 use in there.)  
> >
> > Given the examples should look exactly like those for x86/pc, do we need
> > extra examples in cxl.rst? I guess I can add one simple arm/virt example
> > in a follow up patch without bloating that file too badly..  
> 
> That's fine too -- if the answer is "all these command lines work
> for aarch64 too", then you can just say that in cxl.rst.

I'll put an example in just to avoid people using a default
command line and getting an a55 with too small a PA range.

> 
> > Is the following sufficient for the board specific docs?
> >
> > diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> > index 6a719b9586..10cbffc8a7 100644
> > --- a/docs/system/arm/virt.rst
> > +++ b/docs/system/arm/virt.rst
> > @@ -31,6 +31,7 @@ Supported devices
> >  The virt board supports:
> >
> >  - PCI/PCIe devices
> > +- CXL Fixed memory windows, root bridges and devices.
> >  - Flash memory
> >  - Either one or two PL011 UARTs for the NonSecure World
> >  - An RTC
> > @@ -189,6 +190,14 @@ ras
> >  acpi
> >    Set ``on``/``off``/``auto`` to enable/disable ACPI.
> >
> > +cxl
> > +  Set  ``on``/``off`` to enable/disable CXL. More details in
> > +  :doc:`../devices/cxl`. The default is off.
> > +
> > +cxl-fmw
> > +  Array of CXL fixed memory windows describing fixed address routing to
> > +  target CXL host bridges. See :doc:`../devices/cxl`.
> > +
> >  dtb-randomness
> >    Set ``on``/``off`` to pass random seeds via the guest DTB
> >    rng-seed and kaslr-seed nodes (in both "/chosen" and  
> 
> Looks OK.
> 
> > >  
> > > > @@ -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 */  
> > >
> > > This is going to shuffle the memory map around, even if CXL
> > > isn't enabled, which will break migration compatibility.
> > > You need to do something to ensure that the CXL region isn't
> > > included in the calculations of the base addresses for these
> > > regions if CXL isn't enabled.
> > >  
> >
> > It doesn't move any existing stuff because these are naturally aligned
> > regions so this is in a gap before the PCIE ECAM region.  
> 
> Is that true even when we have the maximum number of CPUs and
> so the max number of redistributors in that VIRT_HIGH_GIC_REDIST2
> region ?

Yes.   The gap is between that and the ECAM window.  The CXL RCRB
stuff doesn't move whether or not that is there.  Making sure it
is present does make it easier to see the gap though - hence example below.


> 
> > > >      [VIRT_HIGH_PCIE_ECAM] =     { 0x0, 256 * MiB },
> > > >      /* Second PCIe window */
> > > >      [VIRT_HIGH_PCIE_MMIO] =     { 0x0, DEFAULT_HIGH_PCIE_MMIO_SIZE },  
> > >
> > > If you're OK with having the CXL host region at the end of the
> > > list then that's a simpler way to avoid its presence disturbing
> > > the layout of the existing regions, but you might not like it
> > > being at such a high physaddr.  
> >
> > From what I recall a higher address isn't a problem I just wanted to not waste any
> > PA space at all so used the gap.
> >
> > Chunk of /proc/iomem with a random test case (in first case with the cxl bits
> > removed as obvious that doesn't start until this patch is in place).
> > Need more than 123 cpus to make the second gicv3 redist appear
> > (I've no idea why that number I just printed the threshold where
> > it was calculated to find out what I needed to wait for boot on).  
> 
> It's 123 because that's the most redistributors we can fit into
> the lower redistributor region. (We didn't really allow enough
> space in the lower memory map, which is why we need this awkward
> split setup.
> 
> (I have to run now, will look at the rest of the email next week.)
Thanks and have a good weekend.

J
> 
> -- PMM

Re: [PATCH v15 3/4] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl

[Jonathan Cameron via]

On Fri, 13 Jun 2025 18:21:25 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Fri, 13 Jun 2025 17:07:24 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> > On Fri, 13 Jun 2025 at 16:20, Jonathan Cameron
> > <Jonathan.Cameron@huawei.com> wrote:  
> > >
> > > On Fri, 13 Jun 2025 13:57:39 +0100
> > > Peter Maydell <peter.maydell@linaro.org> wrote:
> > >    
> > > > On Thu, 12 Jun 2025 at 14:45, Jonathan Cameron
> > > > <Jonathan.Cameron@huawei.com> wrote:    
> > > > >
> > > > > 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>
> > > > >
> > > > > ---
> > > > > v15: No changes.
> > > > > ---
> > > > >  include/hw/arm/virt.h    |  4 ++++
> > > > >  hw/arm/virt-acpi-build.c | 34 ++++++++++++++++++++++++++++++++++
> > > > >  hw/arm/virt.c            | 29 +++++++++++++++++++++++++++++
> > > > >  3 files changed, 67 insertions(+)    
> > > >    
> > > Hi Peter,
> > >
> > > Thanks for reviewing.
> > >    
> > > > Can we have some user-facing documentation, please?
> > > > (docs/system/arm/virt.rst -- can just be a brief mention
> > > > and link to docs/system/devices/cxl.rst if you want to put the
> > > > examples of aarch64 use in there.)    
> > >
> > > Given the examples should look exactly like those for x86/pc, do we need
> > > extra examples in cxl.rst? I guess I can add one simple arm/virt example
> > > in a follow up patch without bloating that file too badly..    
> > 
> > That's fine too -- if the answer is "all these command lines work
> > for aarch64 too", then you can just say that in cxl.rst.  
> 
> I'll put an example in just to avoid people using a default
> command line and getting an a55 with too small a PA range.
> 
> >   
> > > Is the following sufficient for the board specific docs?
> > >
> > > diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> > > index 6a719b9586..10cbffc8a7 100644
> > > --- a/docs/system/arm/virt.rst
> > > +++ b/docs/system/arm/virt.rst
> > > @@ -31,6 +31,7 @@ Supported devices
> > >  The virt board supports:
> > >
> > >  - PCI/PCIe devices
> > > +- CXL Fixed memory windows, root bridges and devices.
> > >  - Flash memory
> > >  - Either one or two PL011 UARTs for the NonSecure World
> > >  - An RTC
> > > @@ -189,6 +190,14 @@ ras
> > >  acpi
> > >    Set ``on``/``off``/``auto`` to enable/disable ACPI.
> > >
> > > +cxl
> > > +  Set  ``on``/``off`` to enable/disable CXL. More details in
> > > +  :doc:`../devices/cxl`. The default is off.
> > > +
> > > +cxl-fmw
> > > +  Array of CXL fixed memory windows describing fixed address routing to
> > > +  target CXL host bridges. See :doc:`../devices/cxl`.
> > > +
> > >  dtb-randomness
> > >    Set ``on``/``off`` to pass random seeds via the guest DTB
> > >    rng-seed and kaslr-seed nodes (in both "/chosen" and    
> > 
> > Looks OK.
> >   
> > > >    
> > > > > @@ -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 */    
> > > >
> > > > This is going to shuffle the memory map around, even if CXL
> > > > isn't enabled, which will break migration compatibility.
> > > > You need to do something to ensure that the CXL region isn't
> > > > included in the calculations of the base addresses for these
> > > > regions if CXL isn't enabled.
> > > >    
> > >
> > > It doesn't move any existing stuff because these are naturally aligned
> > > regions so this is in a gap before the PCIE ECAM region.    
> > 
> > Is that true even when we have the maximum number of CPUs and
> > so the max number of redistributors in that VIRT_HIGH_GIC_REDIST2
> > region ?  
> 
> Yes.   The gap is between that and the ECAM window.  The CXL RCRB
> stuff doesn't move whether or not that is there.  Making sure it
> is present does make it easier to see the gap though - hence example below.
> 
> 
> >   
> > > > >      [VIRT_HIGH_PCIE_ECAM] =     { 0x0, 256 * MiB },
> > > > >      /* Second PCIe window */
> > > > >      [VIRT_HIGH_PCIE_MMIO] =     { 0x0, DEFAULT_HIGH_PCIE_MMIO_SIZE },    
> > > >
> > > > If you're OK with having the CXL host region at the end of the
> > > > list then that's a simpler way to avoid its presence disturbing
> > > > the layout of the existing regions, but you might not like it
> > > > being at such a high physaddr.    
> > >
> > > From what I recall a higher address isn't a problem I just wanted to not waste any
> > > PA space at all so used the gap.
> > >
> > > Chunk of /proc/iomem with a random test case (in first case with the cxl bits
> > > removed as obvious that doesn't start until this patch is in place).
> > > Need more than 123 cpus to make the second gicv3 redist appear
> > > (I've no idea why that number I just printed the threshold where
> > > it was calculated to find out what I needed to wait for boot on).    
> > 
> > It's 123 because that's the most redistributors we can fit into
> > the lower redistributor region. (We didn't really allow enough
> > space in the lower memory map, which is why we need this awkward
> > split setup.
> > 
> > (I have to run now, will look at the rest of the email next week.)  
> Thanks and have a good weekend.

Hi Peter,

I'll post a v16 with the changes discussed and for this patch include a few
comments on what we didn't yet resolve in this thread.

Thanks,

Jonathan

> 
> J
> > 
> > -- PMM  
> 
>