[v9,33/45] cxl/cxl-host: Add memops for CFMWS region.

Commit Message

Jonathan Cameron April 4, 2022, 3:14 p.m. UTC

From: Jonathan Cameron <[e-mail]>

These memops perform interleave decoding, walking down the
CXL topology from CFMWS described host interleave
decoder via CXL host bridge HDM decoders, through the CXL
root ports and finally call CXL type 3 specific read and write
functions.

Note that, whilst functional the current implementation does
not support:
* switches
* multiple HDM decoders at a given level.
* unaligned accesses across the interleave boundaries

Signed-off-by: Jonathan Cameron <[e-mail]>
---
 hw/cxl/cxl-host-stubs.c |   2 +
 hw/cxl/cxl-host.c       | 128 ++++++++++++++++++++++++++++++++++++++++
 include/hw/cxl/cxl.h    |   2 +
 3 files changed, 132 insertions(+)

Comments

Tong Zhang April 7, 2022, 9:07 p.m. UTC | #1

On 4/4/22 08:14, Jonathan Cameron wrote:
> From: Jonathan Cameron <[e-mail]>
>
>
> +static MemTxResult cxl_read_cfmws(void *opaque, hwaddr addr, uint64_t *data,
> +                                  unsigned size, MemTxAttrs attrs)
> +{
> +    CXLFixedWindow *fw = opaque;
> +    PCIDevice *d;
> +
> +    d = cxl_cfmws_find_device(fw, addr);
> +    if (d == NULL) {
> +        *data = 0;

I'm looking at this code and comparing it to CXL2.0 spec 8.2.5.12.2 CXL HDM

Decoder Global Control Register (Offset 04h) table. It seems that we should

check POSION_ON_ERR_EN bit, if this bit is set, we return poison, otherwise

should return all 1's data.

Also, from the spec, this bit is implementation specific and hard 
wired(RO) to either 1 or 0,

but for type3 device looks like we are currently allowing it to be 
overwritten in ct3d_reg_write()

function. We probably also need more sanitation in ct3d_reg_write. (Also 
for HDM

range/interleaving settings.)

> +        /* Reads to invalid address return poison */
> +        return MEMTX_ERROR;
> +    }
> +
> +    return cxl_type3_read(d, addr + fw->base, data, size, attrs);
> +}
> +

- Tong

Jonathan Cameron April 8, 2022, 11:49 a.m. UTC | #2

On Thu, 7 Apr 2022 21:07:06 +0000
Tong Zhang <[e-mail]> wrote:

> On 4/4/22 08:14, Jonathan Cameron wrote:
> > From: Jonathan Cameron <[e-mail]>
> >
> >
> > +static MemTxResult cxl_read_cfmws(void *opaque, hwaddr addr, uint64_t *data,
> > +                                  unsigned size, MemTxAttrs attrs)
> > +{
> > +    CXLFixedWindow *fw = opaque;
> > +    PCIDevice *d;
> > +
> > +    d = cxl_cfmws_find_device(fw, addr);
> > +    if (d == NULL) {
> > +        *data = 0;  
> 
> I'm looking at this code and comparing it to CXL2.0 spec 8.2.5.12.2 CXL HDM
> 
> Decoder Global Control Register (Offset 04h) table. It seems that we should
> 
> check POSION_ON_ERR_EN bit, if this bit is set, we return poison, otherwise
> 
> should return all 1's data.

Good point.  Takes a bit of searching to find the statements on that, but
it should indeed by all 1s not all 0s. I'll fix that up.

> 
> Also, from the spec, this bit is implementation specific and hard 
> wired(RO) to either 1 or 0,

My temptation is to set that to 0 and not return poison, because the handling
of that in the host is horribly implementation specific.

> 
> but for type3 device looks like we are currently allowing it to be 
> overwritten in ct3d_reg_write()
> 
> function. We probably also need more sanitation in ct3d_reg_write. (Also 
> for HDM
> 
> range/interleaving settings.)

Absolutely agree. Generally my plan was to tighten up write restrictions
as a follow on series because it tends to require quite a lot of code and
makes it much harder to see the overall flow.

So far I've done most of the PCI config space santization (see the gitlab
tree) but not much yet on the memory mapped register space.

I'll add it to the todo list. If it turns out this particular case is
reasonably clean I might add it within this series.

Jonathan

> 
> > +        /* Reads to invalid address return poison */
> > +        return MEMTX_ERROR;
> > +    }
> > +
> > +    return cxl_type3_read(d, addr + fw->base, data, size, attrs);
> > +}
> > +  
> 
> - Tong
>

Patch

diff --git a/hw/cxl/cxl-host-stubs.c b/hw/cxl/cxl-host-stubs.c
index f8fd278d5d..24465a52ab 100644
--- a/hw/cxl/cxl-host-stubs.c
+++ b/hw/cxl/cxl-host-stubs.c
@@ -12,3 +12,5 @@  void cxl_fixed_memory_window_config(MachineState *ms,
                                     Error **errp) {};
 
 void cxl_fixed_memory_window_link_targets(Error **errp) {};
+
+const MemoryRegionOps cfmws_ops;
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index ec5a75cbf5..469b3c4ced 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -15,6 +15,10 @@ 
 
 #include "qapi/qapi-visit-machine.h"
 #include "hw/cxl/cxl.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci_bridge.h"
+#include "hw/pci/pci_host.h"
+#include "hw/pci/pcie_port.h"
 
 void cxl_fixed_memory_window_config(MachineState *ms,
                                     CXLFixedMemoryWindowOptions *object,
@@ -92,3 +96,127 @@  void cxl_fixed_memory_window_link_targets(Error **errp)
         }
     }
 }
+
+/* TODO: support, multiple hdm decoders */
+static bool cxl_hdm_find_target(uint32_t *cache_mem, hwaddr addr,
+                                uint8_t *target)
+{
+    uint32_t ctrl;
+    uint32_t ig_enc;
+    uint32_t iw_enc;
+    uint32_t target_reg;
+    uint32_t target_idx;
+
+    ctrl = cache_mem[R_CXL_HDM_DECODER0_CTRL];
+    if (!FIELD_EX32(ctrl, CXL_HDM_DECODER0_CTRL, COMMITTED)) {
+        return false;
+    }
+
+    ig_enc = FIELD_EX32(ctrl, CXL_HDM_DECODER0_CTRL, IG);
+    iw_enc = FIELD_EX32(ctrl, CXL_HDM_DECODER0_CTRL, IW);
+    target_idx = (addr / cxl_decode_ig(ig_enc)) % (1 << iw_enc);
+
+    if (target_idx > 4) {
+        target_reg = cache_mem[R_CXL_HDM_DECODER0_TARGET_LIST_LO];
+        target_reg >>= target_idx * 8;
+    } else {
+        target_reg = cache_mem[R_CXL_HDM_DECODER0_TARGET_LIST_LO];
+        target_reg >>= (target_idx - 4) * 8;
+    }
+    *target = target_reg & 0xff;
+
+    return true;
+}
+
+static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow *fw, hwaddr addr)
+{
+    CXLComponentState *hb_cstate;
+    PCIHostState *hb;
+    int rb_index;
+    uint32_t *cache_mem;
+    uint8_t target;
+    bool target_found;
+    PCIDevice *rp, *d;
+
+    /* Address is relative to memory region. Convert to HPA */
+    addr += fw->base;
+
+    rb_index = (addr / cxl_decode_ig(fw->enc_int_gran)) % fw->num_targets;
+    hb = PCI_HOST_BRIDGE(fw->target_hbs[rb_index]->cxl.cxl_host_bridge);
+    if (!hb || !hb->bus || !pci_bus_is_cxl(hb->bus)) {
+        return NULL;
+    }
+
+    hb_cstate = cxl_get_hb_cstate(hb);
+    if (!hb_cstate) {
+        return NULL;
+    }
+
+    cache_mem = hb_cstate->crb.cache_mem_registers;
+
+    target_found = cxl_hdm_find_target(cache_mem, addr, &target);
+    if (!target_found) {
+        return NULL;
+    }
+
+    rp = pcie_find_port_by_pn(hb->bus, target);
+    if (!rp) {
+        return NULL;
+    }
+
+    d = pci_bridge_get_sec_bus(PCI_BRIDGE(rp))->devices[0];
+
+    if (!d || !object_dynamic_cast(OBJECT(d), TYPE_CXL_TYPE3)) {
+        return NULL;
+    }
+
+    return d;
+}
+
+static MemTxResult cxl_read_cfmws(void *opaque, hwaddr addr, uint64_t *data,
+                                  unsigned size, MemTxAttrs attrs)
+{
+    CXLFixedWindow *fw = opaque;
+    PCIDevice *d;
+
+    d = cxl_cfmws_find_device(fw, addr);
+    if (d == NULL) {
+        *data = 0;
+        /* Reads to invalid address return poison */
+        return MEMTX_ERROR;
+    }
+
+    return cxl_type3_read(d, addr + fw->base, data, size, attrs);
+}
+
+static MemTxResult cxl_write_cfmws(void *opaque, hwaddr addr,
+                                   uint64_t data, unsigned size,
+                                   MemTxAttrs attrs)
+{
+    CXLFixedWindow *fw = opaque;
+    PCIDevice *d;
+
+    d = cxl_cfmws_find_device(fw, addr);
+    if (d == NULL) {
+        /* Writes to invalid address are silent */
+        return MEMTX_OK;
+    }
+
+    return cxl_type3_write(d, addr + fw->base, data, size, attrs);
+}
+
+const MemoryRegionOps cfmws_ops = {
+    .read_with_attrs = cxl_read_cfmws,
+    .write_with_attrs = cxl_write_cfmws,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 8,
+        .unaligned = true,
+    },
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 8,
+        .unaligned = true,
+    },
+};
diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index dce38124db..21d28ca110 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -56,4 +56,6 @@  void cxl_fixed_memory_window_config(MachineState *ms,
                                     Error **errp);
 void cxl_fixed_memory_window_link_targets(Error **errp);
 
+extern const MemoryRegionOps cfmws_ops;
+
 #endif