[v13] arm/virt: CXL support via pxb_cxl

[PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup.

Posted by Jonathan Cameron via 6 months ago

On arm/virt the memory map is set up before any devices are brought
up.  To enable this provide split functions to establish the fw->base
and later to actually map it.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 include/hw/cxl/cxl_host.h |  2 ++
 hw/cxl/cxl-host-stubs.c   |  2 ++
 hw/cxl/cxl-host.c         | 44 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/include/hw/cxl/cxl_host.h b/include/hw/cxl/cxl_host.h
index 6dce2cde07..aee9d573d6 100644
--- a/include/hw/cxl/cxl_host.h
+++ b/include/hw/cxl/cxl_host.h
@@ -16,6 +16,8 @@
 void cxl_machine_init(Object *obj, CXLState *state);
 void cxl_fmws_link_targets(Error **errp);
 void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp);
+hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr);
+void cxl_fmws_update_mmio(void);
 hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr);
 GSList *cxl_fmws_get_all_sorted(void);
 
diff --git a/hw/cxl/cxl-host-stubs.c b/hw/cxl/cxl-host-stubs.c
index 13eb6bf6a4..d9e38618d6 100644
--- a/hw/cxl/cxl-host-stubs.c
+++ b/hw/cxl/cxl-host-stubs.c
@@ -11,6 +11,8 @@
 void cxl_fmws_link_targets(Error **errp) {};
 void cxl_machine_init(Object *obj, CXLState *state) {};
 void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp) {};
+hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr) { return base; };
+void cxl_fmws_update_mmio(void) {};
 hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
 {
     return base;
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index 438f2920e1..3b9184e7e7 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -383,6 +383,7 @@ void cxl_hook_up_pxb_registers(PCIBus *bus, CXLState *state, Error **errp)
 struct cfmw_update_state {
     hwaddr base;
     hwaddr maxaddr;
+    bool update_mmio;
 };
 
 static void cxl_fmws_update(Object *obj, void *opaque)
@@ -397,7 +398,9 @@ static void cxl_fmws_update(Object *obj, void *opaque)
     fw = CXL_FMW(obj);
     if (s->base + fw->size <= s->maxaddr) {
         fw->base = s->base;
-        sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
+        if (s->update_mmio) {
+            sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
+        }
         s->base += fw->size;
     }
 
@@ -438,6 +441,24 @@ GSList *cxl_fmws_get_all_sorted(void)
     return g_slist_sort_with_data(cxl_fmws_get_all(), cfmws_cmp, NULL);
 }
 
+hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr)
+{
+    GSList *cfmws_list, *iter;
+
+    struct cfmw_update_state cfmwss = {
+        .base = base,
+        .maxaddr = max_addr,
+        .update_mmio = false,
+    };
+    cfmws_list = cxl_fmws_get_all_sorted();
+    for (iter = cfmws_list; iter; iter = iter->next) {
+        cxl_fmws_update(iter->data, &cfmwss);
+    }
+    g_slist_free(cfmws_list);
+
+    return cfmwss.base;
+}
+
 hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
 {
     GSList *cfmws_list, *iter;
@@ -445,6 +466,7 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
     struct cfmw_update_state cfmwss = {
         .base = base,
         .maxaddr = max_addr,
+        .update_mmio = true,
     };
     cfmws_list = cxl_fmws_get_all_sorted();
     for (iter = cfmws_list; iter; iter = iter->next) {
@@ -455,6 +477,26 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
     return cfmwss.base;
 }
 
+static int cxl_fmws_mmio_map(Object *obj, void *opaque)
+{
+    struct CXLFixedWindow *fw;
+
+    if (!object_dynamic_cast(obj, TYPE_CXL_FMW)) {
+        return 0;
+    }
+    fw = CXL_FMW(obj);
+    sysbus_mmio_map(SYS_BUS_DEVICE(fw), 0, fw->base);
+
+    return 0;
+}
+
+void cxl_fmws_update_mmio(void)
+{
+    /* Ordering is not required for this */
+    object_child_foreach_recursive(object_get_root(), cxl_fmws_mmio_map,
+                                   NULL);
+}
+
 static void cxl_fmw_realize(DeviceState *dev, Error **errp)
 {
     CXLFixedWindow *fw = CXL_FMW(dev);
-- 
2.43.0

Re: [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup.

Posted by Zhijian Li (Fujitsu) via 6 months ago


On 13/05/2025 19:14, Jonathan Cameron via wrote:
>   
> +hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr)
> +{
> +    GSList *cfmws_list, *iter;
> +
> +    struct cfmw_update_state cfmwss = {
> +        .base = base,
> +        .maxaddr = max_addr,
> +        .update_mmio = false,
> +    };
> +    cfmws_list = cxl_fmws_get_all_sorted();
> +    for (iter = cfmws_list; iter; iter = iter->next) {
> +        cxl_fmws_update(iter->data, &cfmwss);
> +    }
> +    g_slist_free(cfmws_list);
> +
> +    return cfmwss.base;
> +}
> +
>   hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
>   {
>       GSList *cfmws_list, *iter;
> @@ -445,6 +466,7 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
>       struct cfmw_update_state cfmwss = {
>           .base = base,
>           .maxaddr = max_addr,
> +        .update_mmio = true,
>       };
>       cfmws_list = cxl_fmws_get_all_sorted();
>       for (iter = cfmws_list; iter; iter = iter->next) {
> @@ -455,6 +477,26 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
>       return cfmwss.base;
>   }
>   

It seem we can share most of the code in cxl_fmws_set_memmap_and_update_mmio() and cxl_fmws_set_memmap()
In addition, we can drop the cfmw_update_state::update_mmio if there is no other users. Just pass it
to cxl_fmws_update() directly.


Thanks
Zhijian

Re: [PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup.

Posted by Jonathan Cameron via 5 months, 3 weeks ago

On Fri, 16 May 2025 05:50:59 +0000
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> wrote:

> On 13/05/2025 19:14, Jonathan Cameron via wrote:
> >   
> > +hwaddr cxl_fmws_set_memmap(hwaddr base, hwaddr max_addr)
> > +{
> > +    GSList *cfmws_list, *iter;
> > +
> > +    struct cfmw_update_state cfmwss = {
> > +        .base = base,
> > +        .maxaddr = max_addr,
> > +        .update_mmio = false,
> > +    };
> > +    cfmws_list = cxl_fmws_get_all_sorted();
> > +    for (iter = cfmws_list; iter; iter = iter->next) {
> > +        cxl_fmws_update(iter->data, &cfmwss);
> > +    }
> > +    g_slist_free(cfmws_list);
> > +
> > +    return cfmwss.base;
> > +}
> > +
> >   hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> >   {
> >       GSList *cfmws_list, *iter;
> > @@ -445,6 +466,7 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> >       struct cfmw_update_state cfmwss = {
> >           .base = base,
> >           .maxaddr = max_addr,
> > +        .update_mmio = true,
> >       };
> >       cfmws_list = cxl_fmws_get_all_sorted();
> >       for (iter = cfmws_list; iter; iter = iter->next) {
> > @@ -455,6 +477,26 @@ hwaddr cxl_fmws_set_memmap_and_update_mmio(hwaddr base, hwaddr max_addr)
> >       return cfmwss.base;
> >   }
> >     
> 
> It seem we can share most of the code in cxl_fmws_set_memmap_and_update_mmio() and cxl_fmws_set_memmap()
> In addition, we can drop the cfmw_update_state::update_mmio if there is no other users. Just pass it
> to cxl_fmws_update() directly.
I'd like to keep the 'documentation' aspect of the function names, but
can reduce duplication with a do_cxl_fmws_set_mmemap_and_update_mmio() function that
has a shared implementation and takes the extra parameter as you suggestion.

Thanks

Jonathan

> 
> 
> Thanks
> Zhijian

[PATCH v13 1/5] hw/cxl-host: Add an index field to CXLFixedMemoryWindow
[PATCH v13 2/5] hw/cxl: Make the CXL fixed memory windows devices.
[PATCH v13 3/5] hw/cxl-host: Allow split of establishing memory address and mmio setup.
[PATCH v13 4/5] hw/arm/virt: Basic CXL enablement on pci_expander_bridge instances pxb-cxl
[PATCH v13 5/5] qtest/cxl: Add aarch64 virt test for CXL