Re: [PATCH 3/9] cxl/type3: Add dsmas_flags to CXLDCRegion struct

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Mon, 17 Mar 2025 16:31:30 +0000
anisa.su887@gmail.com wrote:

> From: Anisa Su <anisa.su@samsung.com>
> 
> Add dsmas_flags field to DC Region struct in preparation for next
> command, which returns the dsmas flags in the response.
> 
> Signed-off-by: Anisa Su <anisa.su@samsung.com>
> ---
>  hw/mem/cxl_type3.c          | 2 ++
>  include/hw/cxl/cxl_device.h | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 731497ebda..452a0c101a 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -237,6 +237,8 @@ static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv)
>                                            ct3d->dc.regions[i].len,
>                                            false, true, region_base);
>              ct3d->dc.regions[i].dsmadhandle = dsmad_handle - 1;
> +            CDATDsmas *dsmas = (CDATDsmas *) table[cur_ent + CT3_CDAT_DSMAS];
> +            ct3d->dc.regions[i].dsmas_flags = dsmas->flags;

This is relying to much on the ordering of creating fields in
ct3_build_cdat_entries_for_mr().

I'd rather you just stored the information flags is built from in CXLDCRegion
and then built the field that is wonderfully called 'Note' in the DC region
configuration in 6.2 spec.   I've sent a mail to see if we can clean that
'what is the field called' question for future spec releases.

Whilst the flag definitions cross refer the CDAT spec, the actual locations
of those flags matches, but doesn't cross refer so maybe in the future
we will have other flags in here and locations might not match.

>  
>              cur_ent += CT3_CDAT_NUM_ENTRIES;
>              region_base += ct3d->dc.regions[i].len;
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index bebed04085..81b826f570 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -609,6 +609,7 @@ typedef struct CXLDCRegion {
>      uint8_t flags;
>      unsigned long *blk_bitmap;
>      uint64_t supported_blk_size_bitmask;
> +    uint8_t dsmas_flags;
>  } CXLDCRegion;
>  
>  typedef struct CXLSetFeatureInfo {