Re: [PATCH v7 04/12] hw/mem/cxl_type3: Add support to create DC regions to type3 memory devices
On Tue, May 14, 2024 at 08:14:59AM +, Zhijian Li (Fujitsu) wrote:
>
>
> On 19/04/2024 07:10, nifan@gmail.com wrote:
> > From: Fan Ni
> >
>
> > +}
> > +
> > static bool cxl_setup_memory(CXLType3Dev *ct3d, Error **errp)
> > {
> > DeviceState *ds = DEVICE(ct3d);
> > @@ -635,6 +676,13 @@ static bool cxl_setup_memory(CXLType3Dev *ct3d, Error
> > **errp)
> > g_free(p_name);
> > }
> >
> > +if (ct3d->dc.num_regions > 0) {
> > +if (!cxl_create_dc_regions(ct3d, errp)) {
> > +error_setg(errp, "setup DC regions failed");
>
> This error_set() would cause an assertion if the errp was assigned inside
> cxl_create_dc_regions();
> Try error_append_hint() instead
Thanks, Let me check and fix.
Fan
>
> #3 0x7f1fdc4fafc6 in annobin_assert.c_end () at /lib64/libc.so.6
> #4 0x555fd3edbea8 in error_setv
> (errp=0x7ffe6d1a3de0, src=0x555fd3fe262b "../hw/mem/cxl_type3.c",
> line=807, func=0x555fd3fe2fe0 <__func__.21> "cxl_setup_memory",
> err_class=ERROR_CLASS_GENERIC_ERROR, fmt=0x555fd3fe2939 "setup DC regions
> failed", ap=0x7ffe6d1a3
> c00, suffix=0x0) at ../util/error.c:68
> #5 0x555fd3edc126 in error_setg_internal
> (errp=0x7ffe6d1a3de0, src=0x555fd3fe262b "../hw/mem/cxl_type3.c",
> line=807, func=0x555fd3fe2fe0 <__func__.21> "cxl_setup_memory",
> fmt=0x555fd3fe2939 "setup DC regions failed") at ../util/error.c:105
> #6 0x555fd3819c9f in cxl_setup_memory (ct3d=0x555fd8b2f3e0,
> errp=0x7ffe6d1a3de0) at ../hw/mem/cxl_type3.c:807
> #7 0x555fd3819d7b in ct3_realize (pci_dev=0x555fd8b2f3e0,
> errp=0x7ffe6d1a3de0) at ../hw/mem/cxl_type3.c:833
> #8 0x555fd38b575f in pci_qdev_realize (qdev=0x555fd8b2f3e0,
> errp=0x7ffe6d1a3e60) at ../hw/pci/pci.c:2093
> #9 0x555fd3ccca9b in device_set_realized (obj=0x555fd8b2f3e0,
> value=true, errp=0x7ffe6d1a40d0)
Re: [PATCH v7 04/12] hw/mem/cxl_type3: Add support to create DC regions to type3 memory devices
On 19/04/2024 07:10, nifan@gmail.com wrote:
> From: Fan Ni
>
> +}
> +
> static bool cxl_setup_memory(CXLType3Dev *ct3d, Error **errp)
> {
> DeviceState *ds = DEVICE(ct3d);
> @@ -635,6 +676,13 @@ static bool cxl_setup_memory(CXLType3Dev *ct3d, Error
> **errp)
> g_free(p_name);
> }
>
> +if (ct3d->dc.num_regions > 0) {
> +if (!cxl_create_dc_regions(ct3d, errp)) {
> +error_setg(errp, "setup DC regions failed");
This error_set() would cause an assertion if the errp was assigned inside
cxl_create_dc_regions();
Try error_append_hint() instead
#3 0x7f1fdc4fafc6 in annobin_assert.c_end () at /lib64/libc.so.6
#4 0x555fd3edbea8 in error_setv
(errp=0x7ffe6d1a3de0, src=0x555fd3fe262b "../hw/mem/cxl_type3.c",
line=807, func=0x555fd3fe2fe0 <__func__.21> "cxl_setup_memory",
err_class=ERROR_CLASS_GENERIC_ERROR, fmt=0x555fd3fe2939 "setup DC regions
failed", ap=0x7ffe6d1a3
c00, suffix=0x0) at ../util/error.c:68
#5 0x555fd3edc126 in error_setg_internal
(errp=0x7ffe6d1a3de0, src=0x555fd3fe262b "../hw/mem/cxl_type3.c",
line=807, func=0x555fd3fe2fe0 <__func__.21> "cxl_setup_memory",
fmt=0x555fd3fe2939 "setup DC regions failed") at ../util/error.c:105
#6 0x555fd3819c9f in cxl_setup_memory (ct3d=0x555fd8b2f3e0,
errp=0x7ffe6d1a3de0) at ../hw/mem/cxl_type3.c:807
#7 0x555fd3819d7b in ct3_realize (pci_dev=0x555fd8b2f3e0,
errp=0x7ffe6d1a3de0) at ../hw/mem/cxl_type3.c:833
#8 0x555fd38b575f in pci_qdev_realize (qdev=0x555fd8b2f3e0,
errp=0x7ffe6d1a3e60) at ../hw/pci/pci.c:2093
#9 0x555fd3ccca9b in device_set_realized (obj=0x555fd8b2f3e0, value=true,
errp=0x7ffe6d1a40d0)
Re: [PATCH v7 04/12] hw/mem/cxl_type3: Add support to create DC regions to type3 memory devices
On Thu, Apr 18, 2024 at 04:10:55PM -0700, nifan@gmail.com wrote: > From: Fan Ni > > With the change, when setting up memory for type3 memory device, we can > create DC regions. > A property 'num-dc-regions' is added to ct3_props to allow users to pass the > number of DC regions to create. To make it easier, other region parameters > like region base, length, and block size are hard coded. If needed, > these parameters can be added easily. > > With the change, we can create DC regions with proper kernel side > support like below: > > region=$(cat /sys/bus/cxl/devices/decoder0.0/create_dc_region) > echo $region > /sys/bus/cxl/devices/decoder0.0/create_dc_region > echo 256 > /sys/bus/cxl/devices/$region/interleave_granularity > echo 1 > /sys/bus/cxl/devices/$region/interleave_ways > > echo "dc0" >/sys/bus/cxl/devices/decoder2.0/mode > echo 0x4000 >/sys/bus/cxl/devices/decoder2.0/dpa_size > > echo 0x4000 > /sys/bus/cxl/devices/$region/size > echo "decoder2.0" > /sys/bus/cxl/devices/$region/target0 > echo 1 > /sys/bus/cxl/devices/$region/commit > echo $region > /sys/bus/cxl/drivers/cxl_region/bind > > Reviewed-by: Jonathan Cameron > Signed-off-by: Fan Ni > --- > hw/mem/cxl_type3.c | 49 ++ > 1 file changed, 49 insertions(+) > Reviewed-by: Gregory Price
[PATCH v7 04/12] hw/mem/cxl_type3: Add support to create DC regions to type3 memory devices
From: Fan Ni
With the change, when setting up memory for type3 memory device, we can
create DC regions.
A property 'num-dc-regions' is added to ct3_props to allow users to pass the
number of DC regions to create. To make it easier, other region parameters
like region base, length, and block size are hard coded. If needed,
these parameters can be added easily.
With the change, we can create DC regions with proper kernel side
support like below:
region=$(cat /sys/bus/cxl/devices/decoder0.0/create_dc_region)
echo $region > /sys/bus/cxl/devices/decoder0.0/create_dc_region
echo 256 > /sys/bus/cxl/devices/$region/interleave_granularity
echo 1 > /sys/bus/cxl/devices/$region/interleave_ways
echo "dc0" >/sys/bus/cxl/devices/decoder2.0/mode
echo 0x4000 >/sys/bus/cxl/devices/decoder2.0/dpa_size
echo 0x4000 > /sys/bus/cxl/devices/$region/size
echo "decoder2.0" > /sys/bus/cxl/devices/$region/target0
echo 1 > /sys/bus/cxl/devices/$region/commit
echo $region > /sys/bus/cxl/drivers/cxl_region/bind
Reviewed-by: Jonathan Cameron
Signed-off-by: Fan Ni
---
hw/mem/cxl_type3.c | 49 ++
1 file changed, 49 insertions(+)
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 5d6d3ab87d..5ceed0ab4c 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -30,6 +30,7 @@
#include "hw/pci/msix.h"
#define DWORD_BYTE 4
+#define CXL_CAPACITY_MULTIPLIER (256 * MiB)
/* Default CDAT entries for a memory region */
enum {
@@ -567,6 +568,46 @@ static void ct3d_reg_write(void *opaque, hwaddr offset,
uint64_t value,
}
}
+/*
+ * TODO: dc region configuration will be updated once host backend and address
+ * space support is added for DCD.
+ */
+static bool cxl_create_dc_regions(CXLType3Dev *ct3d, Error **errp)
+{
+int i;
+uint64_t region_base = 0;
+uint64_t region_len = 2 * GiB;
+uint64_t decode_len = 2 * GiB;
+uint64_t blk_size = 2 * MiB;
+CXLDCRegion *region;
+MemoryRegion *mr;
+
+if (ct3d->hostvmem) {
+mr = host_memory_backend_get_memory(ct3d->hostvmem);
+region_base += memory_region_size(mr);
+}
+if (ct3d->hostpmem) {
+mr = host_memory_backend_get_memory(ct3d->hostpmem);
+region_base += memory_region_size(mr);
+}
+assert(region_base % CXL_CAPACITY_MULTIPLIER == 0);
+
+for (i = 0, region = >dc.regions[0];
+ i < ct3d->dc.num_regions;
+ i++, region++, region_base += region_len) {
+*region = (CXLDCRegion) {
+.base = region_base,
+.decode_len = decode_len,
+.len = region_len,
+.block_size = blk_size,
+/* dsmad_handle set when creating CDAT table entries */
+.flags = 0,
+};
+}
+
+return true;
+}
+
static bool cxl_setup_memory(CXLType3Dev *ct3d, Error **errp)
{
DeviceState *ds = DEVICE(ct3d);
@@ -635,6 +676,13 @@ static bool cxl_setup_memory(CXLType3Dev *ct3d, Error
**errp)
g_free(p_name);
}
+if (ct3d->dc.num_regions > 0) {
+if (!cxl_create_dc_regions(ct3d, errp)) {
+error_setg(errp, "setup DC regions failed");
+return false;
+}
+}
+
return true;
}
@@ -931,6 +979,7 @@ static Property ct3_props[] = {
HostMemoryBackend *),
DEFINE_PROP_UINT64("sn", CXLType3Dev, sn, UI64_NULL),
DEFINE_PROP_STRING("cdat", CXLType3Dev, cxl_cstate.cdat.filename),
+DEFINE_PROP_UINT8("num-dc-regions", CXLType3Dev, dc.num_regions, 0),
DEFINE_PROP_END_OF_LIST(),
};
--
2.43.0
