On Mon, 4 Mar 2024 11:33:59 -0800
nifan....@gmail.com wrote:
> From: Fan Ni <fan...@samsung.com>
>
> 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 0x40000000 >/sys/bus/cxl/devices/decoder2.0/dpa_size
>
> echo 0x40000000 > /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
>
> Signed-off-by: Fan Ni <fan...@samsung.com>
Suggested changes are trivial formatting things
Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>
> ---
> hw/mem/cxl_type3.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 244d2b5fd5..a191211009 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,45 @@ 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; i < ct3d->dc.num_regions; i++) {
> + region = &ct3d->dc.regions[i];
> + region->base = region_base;
> + region->decode_len = decode_len;
> + region->len = region_len;
> + region->block_size = blk_size;
> + /* dsmad_handle is set when creating cdat table entries */
> + region->flags = 0;
> +
> + region_base += region->len;
Maybe make the loop update to do some or all of the variable updating
(perhaps all of them is a bit too complex!)
for (i = 0, region = &ct3d->dc_regions[0];
i < ct3d->dc.num_regions;
i++, region++, region_base += region_len) {
Also, using this style of assignment will avoid lots of repetition of region.
*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;
> +}
