[RFC PATCH] hw/mem/cxl_type3: Go back to subregions

Ben Widawsky posted 1 patch 3 years, 1 month ago
Failed in applying to current master (apply log)
hw/mem/cxl_type3.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
[RFC PATCH] hw/mem/cxl_type3: Go back to subregions
Posted by Ben Widawsky 3 years, 1 month ago
Each device allocates its memory (persistent only for now) out of a
container memory that represents a "window" that would be defined by the
host bridge. For example, a host bridge may claim all traffic from 0x0 -
0x4000; it might then also direct 0x1000-0x1fff to a specific CXL
device. Change the memory region type claiming 0x1000-0x1fff from an
alias, to a subregion.

v1 and v2 of the patch series both used a subregion. There were two
issues with this and so for v3, an alias was chosen mimicking nvdimm.

The switch to alias left an issue in the implementation. There's logic
that check to make sure two memory regions (ie. two devices under the
same host bridge) couldn't collide. While hardware doesn't make this
guarantee, it's nice for driver debug. There is no clean way to do that
with an alias.

More importantly, this change was inspired by implementing support for
both volatile and persistent memory. In that case, you may have multiple
devices which the BIOS is going to assign address ranges to. Since we
are the BIOS in this case, having a way of finding used space in the
memory window so that you can allocate the next chunk is easily
accomplished here.

With this, I have the following output from `info mtree`
    0000004c00000000-0000004c1fffffff (prio 0, ram): cxl-mem1
      0000004c00000000-0000004c0fffffff (prio 1, i/o): cxl_type3-memory

And `info memory-devices`
Memory device [cxl]: "cxl-pmem0"
  addr: 0x4c00000000
  slot: 0
  node: 0
  size: 268435456
  memdev: /objects/cxl-mem1
  hotplugged: false
  hotpluggable: true

This functionality has been tested with a WIP linux patch which amounts
to this:
       reg = readl(cxlm->regs.hdm_decoder + CXL_HDM_DECODER_CAP_OFFSET);

       dev_err(dev, "decoder cap:\n"
                    "\tcount: %lu\n",
               FIELD_GET(CXL_HDM_DECODER_COUNT_MASK, reg));

       writel(0x4c, cxlm->regs.hdm_decoder + CXL_HDM_DECODER0_BASE_HIGH_OFFSET);
       writel(0, cxlm->regs.hdm_decoder + CXL_HDM_DECODER0_BASE_LOW_OFFSET);
       writel(0, cxlm->regs.hdm_decoder + CXL_HDM_DECODER0_SIZE_HIGH_OFFSET);
       writel(256 << 20, cxlm->regs.hdm_decoder + CXL_HDM_DECODER0_SIZE_LOW_OFFSET);
       writel(BIT(9),  cxlm->regs.hdm_decoder + CXL_HDM_DECODER0_CTRL_OFFSET);

       tmp = ioremap_uc(0x4c00000000, 4096);
       writel(0x20, tmp);

Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 hw/mem/cxl_type3.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index bf33ddb915..33991079a6 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -46,7 +46,9 @@ static void build_dvsecs(CXLType3Dev *ct3d)
 static void cxl_set_addr(CXLType3Dev *ct3d, hwaddr addr, Error **errp)
 {
     MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(ct3d);
-    mdc->set_addr(MEMORY_DEVICE(ct3d), addr, errp);
+    MemoryRegion *mr = host_memory_backend_get_memory(ct3d->hostmem);
+
+    mdc->set_addr(MEMORY_DEVICE(ct3d), addr - mr->addr, errp);
 }
 
 static void hdm_decoder_commit(CXLType3Dev *ct3d, int which)
@@ -180,13 +182,13 @@ static void cxl_setup_memory(CXLType3Dev *ct3d, Error **errp)
 
     memory_region_set_nonvolatile(pmem, true);
     memory_region_set_enabled(pmem, false);
-    memory_region_init_alias(pmem, OBJECT(ct3d), "cxl_type3-memory", mr, 0,
-                             ct3d->size);
+    memory_region_init(pmem, OBJECT(ct3d), "cxl_type3-memory", ct3d->size);
+    memory_region_add_subregion_overlap(mr, 0, pmem, 1);
     ct3d->cxl_dstate.pmem = pmem;
 
 #ifdef SET_PMEM_PADDR
     /* This path will initialize the memory device as if BIOS had done it */
-    cxl_set_addr(ct3d, mr->addr + offset, errp);
+    cxl_set_addr(ct3d, offset, errp);
     memory_region_set_enabled(pmem, true);
 #endif
 }
@@ -246,8 +248,11 @@ static uint64_t cxl_md_get_addr(const MemoryDeviceState *md)
     CXLType3Dev *ct3d = CT3(md);
     MemoryRegion *pmem = ct3d->cxl_dstate.pmem;
 
-    assert(pmem->alias);
-    return pmem->alias_offset;
+    if (pmem) {
+        return pmem->addr + pmem->container->addr;
+    }
+
+    return 0;
 }
 
 static void cxl_md_set_addr(MemoryDeviceState *md, uint64_t addr, Error **errp)
@@ -255,8 +260,6 @@ static void cxl_md_set_addr(MemoryDeviceState *md, uint64_t addr, Error **errp)
     CXLType3Dev *ct3d = CT3(md);
     MemoryRegion *pmem = ct3d->cxl_dstate.pmem;
 
-    assert(pmem->alias);
-    memory_region_set_alias_offset(pmem, addr);
     memory_region_set_address(pmem, addr);
 }
 
-- 
2.30.2