From: Anisa Su <anisa.su@samsung.com>
Add supported_blk_size field to CXLDCRegion struct in preparation for
next patch. It is needed by command 0x5600 Get DC Region Config.
Signed-off-by: Anisa Su <anisa.su@samsung.com>
---
hw/mem/cxl_type3.c | 3 +++
include/hw/cxl/cxl_device.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 11c38a9292..731497ebda 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -8,6 +8,7 @@
*
* SPDX-License-Identifier: GPL-v2-only
*/
+#include <math.h>
#include "qemu/osdep.h"
#include "qemu/units.h"
@@ -766,6 +767,7 @@ static bool cxl_create_dc_regions(CXLType3Dev *ct3d, Error **errp)
uint64_t region_len;
uint64_t decode_len;
uint64_t blk_size = 2 * MiB;
+ uint64_t supported_blk_size_bitmask = BIT((int) log2(blk_size));
CXLDCRegion *region;
MemoryRegion *mr;
uint64_t dc_size;
@@ -811,6 +813,7 @@ static bool cxl_create_dc_regions(CXLType3Dev *ct3d, Error **errp)
.block_size = blk_size,
/* dsmad_handle set when creating CDAT table entries */
.flags = 0,
+ .supported_blk_size_bitmask = supported_blk_size_bitmask,
};
ct3d->dc.total_capacity += region->len;
region->blk_bitmap = bitmap_new(region->len / region->block_size);
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index ca515cab13..bebed04085 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -608,6 +608,7 @@ typedef struct CXLDCRegion {
uint32_t dsmadhandle;
uint8_t flags;
unsigned long *blk_bitmap;
+ uint64_t supported_blk_size_bitmask;
} CXLDCRegion;
typedef struct CXLSetFeatureInfo {
--
2.47.2
On Mon, 17 Mar 2025 16:31:28 +0000
anisa.su887@gmail.com wrote:
> From: Anisa Su <anisa.su@samsung.com>
>
> Add supported_blk_size field to CXLDCRegion struct in preparation for
> next patch. It is needed by command 0x5600 Get DC Region Config.
Hi Anisa,
Sorry it took me so long to your series!
Squash this with patch 2. It isn't complex enough addition to justify
as separate patch where we need to explain it is there for that patch
in the cover letter.
>
> Signed-off-by: Anisa Su <anisa.su@samsung.com>
> ---
> hw/mem/cxl_type3.c | 3 +++
> include/hw/cxl/cxl_device.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 11c38a9292..731497ebda 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -8,6 +8,7 @@
> *
> * SPDX-License-Identifier: GPL-v2-only
> */
> +#include <math.h>
>
> #include "qemu/osdep.h"
> #include "qemu/units.h"
> @@ -766,6 +767,7 @@ static bool cxl_create_dc_regions(CXLType3Dev *ct3d, Error **errp)
> uint64_t region_len;
> uint64_t decode_len;
> uint64_t blk_size = 2 * MiB;
> + uint64_t supported_blk_size_bitmask = BIT((int) log2(blk_size));
Isn't this going in circles? I guess it sort of acts as documentation that it
is a bitmap but then the name is already doing that.
Maybe set it to blk_size and add a comment that for now only a fixed block size
is supported?
I'm a little confused on what this is for given you don't check it in patch 6
which is changing the block size?
> CXLDCRegion *region;
> MemoryRegion *mr;
> uint64_t dc_size;
> @@ -811,6 +813,7 @@ static bool cxl_create_dc_regions(CXLType3Dev *ct3d, Error **errp)
> .block_size = blk_size,
> /* dsmad_handle set when creating CDAT table entries */
> .flags = 0,
> + .supported_blk_size_bitmask = supported_blk_size_bitmask,
> };
> ct3d->dc.total_capacity += region->len;
> region->blk_bitmap = bitmap_new(region->len / region->block_size);
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index ca515cab13..bebed04085 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -608,6 +608,7 @@ typedef struct CXLDCRegion {
> uint32_t dsmadhandle;
> uint8_t flags;
> unsigned long *blk_bitmap;
> + uint64_t supported_blk_size_bitmask;
> } CXLDCRegion;
>
> typedef struct CXLSetFeatureInfo {
On Thu, Apr 24, 2025 at 11:11:31AM +0100, Jonathan Cameron wrote:
> On Mon, 17 Mar 2025 16:31:28 +0000
> anisa.su887@gmail.com wrote:
>
> > From: Anisa Su <anisa.su@samsung.com>
> >
> > Add supported_blk_size field to CXLDCRegion struct in preparation for
> > next patch. It is needed by command 0x5600 Get DC Region Config.
...
> > @@ -8,6 +8,7 @@
> > *
> > * SPDX-License-Identifier: GPL-v2-only
> > */
> > +#include <math.h>
> >
> > #include "qemu/osdep.h"
> > #include "qemu/units.h"
> > @@ -766,6 +767,7 @@ static bool cxl_create_dc_regions(CXLType3Dev *ct3d, Error **errp)
> > uint64_t region_len;
> > uint64_t decode_len;
> > uint64_t blk_size = 2 * MiB;
> > + uint64_t supported_blk_size_bitmask = BIT((int) log2(blk_size));
>
> Isn't this going in circles? I guess it sort of acts as documentation that it
> is a bitmap but then the name is already doing that.
> Maybe set it to blk_size and add a comment that for now only a fixed block size
> is supported?
>
> I'm a little confused on what this is for given you don't check it in patch 6
> which is changing the block size?
Good catch! It doesn't seem to specify this check in Section 7.6.7.6.3
Set DC Region Configuration (Opcode 5602h) in the 3.2 spec, but it would
make sense to me to fail with the same rc as when the region has not
been cleared, which is CXL_MBOX_UNSUPPORTED.
Will have the fix in v2.
> > CXLDCRegion *region;
> > MemoryRegion *mr;
> > uint64_t dc_size;
> > @@ -811,6 +813,7 @@ static bool cxl_create_dc_regions(CXLType3Dev *ct3d, Error **errp)
> > .block_size = blk_size,
> > /* dsmad_handle set when creating CDAT table entries */
> > .flags = 0,
> > + .supported_blk_size_bitmask = supported_blk_size_bitmask,
> > };
> > ct3d->dc.total_capacity += region->len;
> > region->blk_bitmap = bitmap_new(region->len / region->block_size);
> > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> > index ca515cab13..bebed04085 100644
> > --- a/include/hw/cxl/cxl_device.h
> > +++ b/include/hw/cxl/cxl_device.h
> > @@ -608,6 +608,7 @@ typedef struct CXLDCRegion {
> > uint32_t dsmadhandle;
> > uint8_t flags;
> > unsigned long *blk_bitmap;
> > + uint64_t supported_blk_size_bitmask;
> > } CXLDCRegion;
> >
> > typedef struct CXLSetFeatureInfo {
>
© 2016 - 2026 Red Hat, Inc.